From 871ee1cc6cbc23b82fd984219fbe1a0b9568c149 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Sun, 9 Feb 2014 22:02:23 -0500 Subject: [PATCH 01/25] IMAP authentication improvements Changes: Implement the PLAIN SASL mechanism. IMAPv4rev1 assures its availability so long as the connection is encrypted. The big advantage of PLAIN over IMAP "LOGIN" is that PLAIN uses UTF-8 encoding for the user name and password, whereas "LOGIN" is only safe for 7-bit US-ASCII -- the encoding of 8-bit data is undefined. (Note that RFC 6855 says that IMAP "LOGIN" does not support UTF-8, and clients must use IMAP "AUTHENTICATE" to pass UTF-8 user names and passwords.) Honor the "LOGINDISABLED" CAPABILITY (RFC 2595) when the server declares it. There's no sense transmitting a password in the clear when it is known that it will be rejected. No attempt is made to try CRAM-MD5 if the server doesn't profess to support it in its CAPABILITY response. (This is the same behavior as Thunderbird.) Extract code from ImapConnection.open into new method ImapConnection.login. Extract code from ImapConnection.executeSimpleCommand into new method ImapConnection.readStatusResponse. Related issues: 6015, 6016 --- src/com/fsck/k9/mail/store/ImapStore.java | 152 ++++++++++++++++------ 1 file changed, 111 insertions(+), 41 deletions(-) diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index 87a0c2fb4..c10bb67a5 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -84,6 +84,7 @@ import com.fsck.k9.mail.PushReceiver; import com.fsck.k9.mail.Pusher; import com.fsck.k9.mail.ServerSettings; import com.fsck.k9.mail.Store; +import com.fsck.k9.mail.filter.Base64; import com.fsck.k9.mail.filter.EOLConvertingOutputStream; import com.fsck.k9.mail.filter.FixedLengthInputStream; import com.fsck.k9.mail.filter.PeekableInputStream; @@ -128,6 +129,9 @@ public class ImapStore extends Store { private Set mPermanentFlagsIndex = new HashSet(); private static final String CAPABILITY_IDLE = "IDLE"; + private static final String CAPABILITY_AUTH_CRAM_MD5 = "AUTH=CRAM-MD5"; + private static final String CAPABILITY_AUTH_PLAIN = "AUTH=PLAIN"; + private static final String CAPABILITY_LOGINDISABLED = "LOGINDISABLED"; private static final String COMMAND_IDLE = "IDLE"; private static final String CAPABILITY_NAMESPACE = "NAMESPACE"; private static final String COMMAND_NAMESPACE = "NAMESPACE"; @@ -2517,6 +2521,14 @@ public class ImapStore extends Store { .getInputStream(), 1024)); mParser = new ImapResponseParser(mIn); mOut = mSocket.getOutputStream(); + // Per RFC 2595 (3.1): Once TLS has been started, reissue CAPABILITY command + if (K9.DEBUG) + Log.i(K9.LOG_TAG, "Updating capabilities after STARTTLS for " + getLogId()); + capabilities.clear(); + List responses = receiveCapabilities(executeSimpleCommand(COMMAND_CAPABILITY)); + if (responses.size() != 2) { + throw new MessagingException("Invalid CAPABILITY response received"); + } } else if (mSettings.getConnectionSecurity() == CONNECTION_SECURITY_TLS_REQUIRED) { throw new MessagingException("TLS not supported but required"); } @@ -2525,20 +2537,30 @@ public class ImapStore extends Store { mOut = new BufferedOutputStream(mOut, 1024); try { - if (mSettings.getAuthType() == AuthType.CRAM_MD5) { - authCramMD5(); - // The authCramMD5 method called on the previous line does not allow for handling updated capabilities - // sent by the server. So, to make sure we update to the post-authentication capability list - // we fetch the capabilities here. - if (K9.DEBUG) - Log.i(K9.LOG_TAG, "Updating capabilities after CRAM-MD5 authentication for " + getLogId()); - List responses = receiveCapabilities(executeSimpleCommand(COMMAND_CAPABILITY)); - if (responses.size() != 2) { - throw new MessagingException("Invalid CAPABILITY response received"); + switch (mSettings.getAuthType()) { + case CRAM_MD5: + if (hasCapability(CAPABILITY_AUTH_CRAM_MD5)) { + authCramMD5(); + } else { + throw new MessagingException( + "Server doesn't support encrypted passwords using CRAM-MD5."); } + break; - } else if (mSettings.getAuthType() == AuthType.PLAIN) { - receiveCapabilities(executeSimpleCommand(String.format("LOGIN %s %s", ImapStore.encodeString(mSettings.getUsername()), ImapStore.encodeString(mSettings.getPassword())), true)); + case PLAIN: + if (hasCapability(CAPABILITY_AUTH_PLAIN)) { + saslAuthPlain(); + } else if (!hasCapability(CAPABILITY_LOGINDISABLED)) { + login(); + } else { + throw new MessagingException( + "Server doesn't support unencrypted passwords using AUTH=PLAIN and LOGIN is disabled."); + } + break; + + default: + throw new MessagingException( + "Unhandled authentication method found in the server settings (bug)."); } authSuccess = true; } catch (ImapException ie) { @@ -2664,6 +2686,13 @@ public class ImapStore extends Store { } } + protected void login() throws IOException, MessagingException { + receiveCapabilities(executeSimpleCommand(String.format( + "LOGIN %s %s", + ImapStore.encodeString(mSettings.getUsername()), + ImapStore.encodeString(mSettings.getPassword())), true)); + } + protected void authCramMD5() throws AuthenticationFailedException, MessagingException { try { String tag = sendCommand("AUTHENTICATE CRAM-MD5", false); @@ -2708,6 +2737,75 @@ public class ImapStore extends Store { } } + protected void saslAuthPlain() throws IOException, MessagingException { + String command = "AUTHENTICATE PLAIN"; + String tag = sendCommand(command, false); + readContinuationResponse(tag); + mOut.write(Base64.encodeBase64(("\000" + mSettings.getUsername() + + "\000" + mSettings.getPassword()).getBytes())); + mOut.write('\r'); + mOut.write('\n'); + mOut.flush(); + try { + receiveCapabilities(readStatusResponse(tag, command, null)); + } catch (MessagingException e) { + throw new AuthenticationFailedException(e.getMessage()); + } + } + + protected ImapResponse readContinuationResponse(String tag) + throws IOException, MessagingException { + ImapResponse response; + do { + response = readResponse(); + if (response.mTag != null) { + if (response.mTag.equalsIgnoreCase(tag)) { + throw new MessagingException( + "Command continuation aborted: " + response); + } else { + Log.w(K9.LOG_TAG, "After sending tag " + tag + + ", got tag response from previous command " + + response + " for " + getLogId()); + } + } + } while (!response.mCommandContinuationRequested); + return response; + } + + protected ArrayList readStatusResponse(String tag, + String commandToLog, UntaggedHandler untaggedHandler) + throws IOException, MessagingException { + ArrayList responses = new ArrayList(); + ImapResponse response; + do { + response = mParser.readResponse(); + if (K9.DEBUG && K9.DEBUG_PROTOCOL_IMAP) + Log.v(K9.LOG_TAG, getLogId() + "<<<" + response); + + if (response.mTag != null && !response.mTag.equalsIgnoreCase(tag)) { + Log.w(K9.LOG_TAG, "After sending tag " + tag + ", got tag response from previous command " + response + " for " + getLogId()); + Iterator iter = responses.iterator(); + while (iter.hasNext()) { + ImapResponse delResponse = iter.next(); + if (delResponse.mTag != null || delResponse.size() < 2 + || (!ImapResponseParser.equalsIgnoreCase(delResponse.get(1), "EXISTS") && !ImapResponseParser.equalsIgnoreCase(delResponse.get(1), "EXPUNGE"))) { + iter.remove(); + } + } + response.mTag = null; + continue; + } + if (untaggedHandler != null) { + untaggedHandler.handleAsyncUntaggedResponse(response); + } + responses.add(response); + } while (response.mTag == null); + if (response.size() < 1 || !ImapResponseParser.equalsIgnoreCase(response.get(0), "OK")) { + throw new ImapException("Command: " + commandToLog + "; response: " + response.toString(), response.getAlertText()); + } + return responses; + } + protected void setReadTimeout(int millis) throws SocketException { Socket sock = mSocket; if (sock != null) { @@ -2832,35 +2930,7 @@ public class ImapStore extends Store { //if (K9.DEBUG) // Log.v(K9.LOG_TAG, "Sent IMAP command " + commandToLog + " with tag " + tag + " for " + getLogId()); - ArrayList responses = new ArrayList(); - ImapResponse response; - do { - response = mParser.readResponse(); - if (K9.DEBUG && K9.DEBUG_PROTOCOL_IMAP) - Log.v(K9.LOG_TAG, getLogId() + "<<<" + response); - - if (response.mTag != null && !response.mTag.equalsIgnoreCase(tag)) { - Log.w(K9.LOG_TAG, "After sending tag " + tag + ", got tag response from previous command " + response + " for " + getLogId()); - Iterator iter = responses.iterator(); - while (iter.hasNext()) { - ImapResponse delResponse = iter.next(); - if (delResponse.mTag != null || delResponse.size() < 2 - || (!ImapResponseParser.equalsIgnoreCase(delResponse.get(1), "EXISTS") && !ImapResponseParser.equalsIgnoreCase(delResponse.get(1), "EXPUNGE"))) { - iter.remove(); - } - } - response.mTag = null; - continue; - } - if (untaggedHandler != null) { - untaggedHandler.handleAsyncUntaggedResponse(response); - } - responses.add(response); - } while (response.mTag == null); - if (response.size() < 1 || !ImapResponseParser.equalsIgnoreCase(response.get(0), "OK")) { - throw new ImapException("Command: " + commandToLog + "; response: " + response.toString(), response.getAlertText()); - } - return responses; + return readStatusResponse(tag, commandToLog, untaggedHandler); } } From 1d1b14da21cd4dd08264cf2d98fa7464605128a9 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Mon, 28 Oct 2013 18:24:53 -0400 Subject: [PATCH 02/25] Fix ImapStore$ImapConnection.authCramMD5() See Issue 4492 This method made way too many assumptions about server responses and should not have been attempting to read and parse them. That should be left to ImapResponseParser. --- src/com/fsck/k9/mail/store/ImapStore.java | 58 +++++++---------------- 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index c10bb67a5..566c926af 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -2693,47 +2693,25 @@ public class ImapStore extends Store { ImapStore.encodeString(mSettings.getPassword())), true)); } - protected void authCramMD5() throws AuthenticationFailedException, MessagingException { + protected void authCramMD5() throws MessagingException, IOException { + String command = "AUTHENTICATE CRAM-MD5"; + String tag = sendCommand(command, false); + ImapResponse response = readContinuationResponse(tag); + if (response.size() != 1 || !(response.get(0) instanceof String)) { + throw new MessagingException("Invalid Cram-MD5 nonce received"); + } + byte[] b64Nonce = response.getString(0).getBytes(); + byte[] b64CRAM = Authentication.computeCramMd5Bytes( + mSettings.getUsername(), mSettings.getPassword(), b64Nonce); + + mOut.write(b64CRAM); + mOut.write('\r'); + mOut.write('\n'); + mOut.flush(); try { - String tag = sendCommand("AUTHENTICATE CRAM-MD5", false); - byte[] buf = new byte[1024]; - int b64NonceLen = 0; - for (int i = 0; i < buf.length; i++) { - buf[i] = (byte)mIn.read(); - if (buf[i] == 0x0a) { - b64NonceLen = i; - break; - } - } - if (b64NonceLen == 0) { - throw new AuthenticationFailedException("Error negotiating CRAM-MD5: nonce too long."); - } - byte[] b64NonceTrim = new byte[b64NonceLen - 2]; - System.arraycopy(buf, 1, b64NonceTrim, 0, b64NonceLen - 2); - - byte[] b64CRAM = Authentication.computeCramMd5Bytes(mSettings.getUsername(), - mSettings.getPassword(), b64NonceTrim); - - mOut.write(b64CRAM); - mOut.write(new byte[] { 0x0d, 0x0a }); - mOut.flush(); - - int respLen = 0; - for (int i = 0; i < buf.length; i++) { - buf[i] = (byte)mIn.read(); - if (buf[i] == 0x0a) { - respLen = i; - break; - } - } - - String toMatch = tag + " OK"; - String respStr = new String(buf, 0, respLen); - if (!respStr.startsWith(toMatch)) { - throw new AuthenticationFailedException("CRAM-MD5 error: " + respStr); - } - } catch (IOException ioe) { - throw new AuthenticationFailedException("CRAM-MD5 Auth Failed.", ioe); + receiveCapabilities(readStatusResponse(tag, command, null)); + } catch (MessagingException e) { + throw new AuthenticationFailedException(e.getMessage()); } } From 6f49ebd975a145734bc5705b34056228ce5a2161 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Mon, 24 Feb 2014 11:58:30 -0500 Subject: [PATCH 03/25] Permit UTF-8 names & passwords with CRAM-MD5 authentication CRAM-MD5 (RFC 2195) permits 8-bit data but does not identify its encoding. Since ASCII does not permit 8-bit data, this commit changes the encoding to UTF-8. There is an expired Internet-Draft that proposed that the RFC be changed to explicitly require UTF-8 encoding of user names and shared secrets. (But then there's also an expired draft proposing that CRAM-MD5 be retired to historic status.) Instead of CRAM-MD5, a better option for users is the SASL PLAIN mechanism (within TLS) which explicitly permits UTF-8. --- src/com/fsck/k9/mail/Authentication.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/com/fsck/k9/mail/Authentication.java b/src/com/fsck/k9/mail/Authentication.java index 4b9ae90ae..a4a9b4fbf 100644 --- a/src/com/fsck/k9/mail/Authentication.java +++ b/src/com/fsck/k9/mail/Authentication.java @@ -54,7 +54,7 @@ public class Authentication { try { byte[] nonce = Base64.decodeBase64(b64Nonce); - byte[] secretBytes = password.getBytes(US_ASCII); + byte[] secretBytes = password.getBytes(); MessageDigest md = MessageDigest.getInstance("MD5"); if (secretBytes.length > 64) { secretBytes = md.digest(secretBytes); @@ -74,7 +74,7 @@ public class Authentication { byte[] result = md.digest(firstPass); String plainCRAM = username + " " + new String(Hex.encodeHex(result)); - byte[] b64CRAM = Base64.encodeBase64(plainCRAM.getBytes(US_ASCII)); + byte[] b64CRAM = Base64.encodeBase64(plainCRAM.getBytes()); return b64CRAM; From 3877f58515bb6a5fd2c9cfddeaa59232a65caf94 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Mon, 24 Feb 2014 11:58:41 -0500 Subject: [PATCH 04/25] (partially) Fix IMAP LOGIN Previously, the user name and password were being transmitted as IMAP (RFC 3501) quoted strings. The problem is that quoted strings are only permitted to carry 7-bit (ASCII) data, whereas user names and passwords entered in K-9 Mail may not be ASCII, so K-9 was violating the RFC by sending them as quoted strings. The solution is to transmit the credentials as IMAP literal strings, which are permitted for user names and passwords, and which permit the transmission of 8-bit data. This is only a partial attempt for fixing the LOGIN command for users with non-ASCII credentials. The problem is that IMAP permits 8-bit data for user names and passwords (if transmitted as literals), but the RFC says nothing about the character encoding for 8-bit data. This commit encodes them as UTF-8. The RFC author's comments on the subject: http://mailman2.u.washington.edu/pipermail/imap-protocol/2008-February/000822.html Ideally, users should avoid the LOGIN command and use the SASL PLAIN mechanism (within TLS) which explicitly permits UTF-8. (K-9 Mail always chooses PLAIN over LOGIN, when PLAIN is available.) --- src/com/fsck/k9/mail/store/ImapStore.java | 30 ++++++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index 566c926af..0d4d077bc 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -132,6 +132,7 @@ public class ImapStore extends Store { private static final String CAPABILITY_AUTH_CRAM_MD5 = "AUTH=CRAM-MD5"; private static final String CAPABILITY_AUTH_PLAIN = "AUTH=PLAIN"; private static final String CAPABILITY_LOGINDISABLED = "LOGINDISABLED"; + private static final String CAPABILITY_LITERAL_PLUS = "LITERAL+"; private static final String COMMAND_IDLE = "IDLE"; private static final String CAPABILITY_NAMESPACE = "NAMESPACE"; private static final String COMMAND_NAMESPACE = "NAMESPACE"; @@ -2687,10 +2688,31 @@ public class ImapStore extends Store { } protected void login() throws IOException, MessagingException { - receiveCapabilities(executeSimpleCommand(String.format( - "LOGIN %s %s", - ImapStore.encodeString(mSettings.getUsername()), - ImapStore.encodeString(mSettings.getPassword())), true)); + boolean hasLiteralPlus = hasCapability(CAPABILITY_LITERAL_PLUS); + String tag; + byte[] username = mSettings.getUsername().getBytes(); + byte[] password = mSettings.getPassword().getBytes(); + tag = sendCommand(String.format("LOGIN {%d%s}", + username.length, hasLiteralPlus ? "+" : ""), true); + if (!hasLiteralPlus) { + readContinuationResponse(tag); + } + mOut.write(username); + mOut.write(String.format(" {%d%s}\r\n", password.length, + hasLiteralPlus ? "+" : "").getBytes()); + if (!hasLiteralPlus) { + mOut.flush(); + readContinuationResponse(tag); + } + mOut.write(password); + mOut.write('\r'); + mOut.write('\n'); + mOut.flush(); + try { + receiveCapabilities(readStatusResponse(tag, "LOGIN", null)); + } catch (MessagingException e) { + throw new AuthenticationFailedException(e.getMessage()); + } } protected void authCramMD5() throws MessagingException, IOException { From 26491676faee56011d9c255d78f0565e4b14f907 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Sat, 22 Feb 2014 17:18:31 -0500 Subject: [PATCH 05/25] Retrict use of AuthenticationFailedException In AccountSetupCheckSettings.onCreate(Bundle), the account settings are checked. If an AuthenticationFailedException occurs, A dialog saying "Username or password incorrect." pops up. We don't want to say this if the cause is not related to an incorrect user name or password. Instead we want to say the more generic "Cannot connect to server" which pops up for other exception types. This commit attempts to eliminate the use of AuthenticationFailedException in instances where it could not be due to "Username or password incorrect." --- src/com/fsck/k9/mail/Authentication.java | 14 ++--- src/com/fsck/k9/mail/store/ImapStore.java | 51 ++++++++----------- src/com/fsck/k9/mail/store/Pop3Store.java | 20 ++++---- .../fsck/k9/mail/transport/SmtpTransport.java | 6 +-- 4 files changed, 43 insertions(+), 48 deletions(-) diff --git a/src/com/fsck/k9/mail/Authentication.java b/src/com/fsck/k9/mail/Authentication.java index a4a9b4fbf..2872b1b58 100644 --- a/src/com/fsck/k9/mail/Authentication.java +++ b/src/com/fsck/k9/mail/Authentication.java @@ -17,21 +17,21 @@ public class Authentication { * @param b64Nonce The nonce as base64-encoded string. * @return The CRAM-MD5 response. * - * @throws AuthenticationFailedException If something went wrong. + * @throws MessagingException If something went wrong. * * @see Authentication#computeCramMd5Bytes(String, String, byte[]) */ public static String computeCramMd5(String username, String password, String b64Nonce) - throws AuthenticationFailedException { + throws MessagingException { try { byte[] b64NonceBytes = b64Nonce.getBytes(US_ASCII); byte[] b64CRAM = computeCramMd5Bytes(username, password, b64NonceBytes); return new String(b64CRAM, US_ASCII); - } catch (AuthenticationFailedException e) { + } catch (MessagingException e) { throw e; } catch (Exception e) { - throw new AuthenticationFailedException("This shouldn't happen", e); + throw new MessagingException("This shouldn't happen", e); } } @@ -44,12 +44,12 @@ public class Authentication { * @param b64Nonce The nonce as base64-encoded byte array. * @return The CRAM-MD5 response as byte array. * - * @throws AuthenticationFailedException If something went wrong. + * @throws MessagingException If something went wrong. * * @see RFC 2195 */ public static byte[] computeCramMd5Bytes(String username, String password, byte[] b64Nonce) - throws AuthenticationFailedException { + throws MessagingException { try { byte[] nonce = Base64.decodeBase64(b64Nonce); @@ -79,7 +79,7 @@ public class Authentication { return b64CRAM; } catch (Exception e) { - throw new AuthenticationFailedException("Something went wrong during CRAM-MD5 computation", e); + throw new MessagingException("Something went wrong during CRAM-MD5 computation", e); } } } diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index 0d4d077bc..f557a852e 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -2537,39 +2537,32 @@ public class ImapStore extends Store { mOut = new BufferedOutputStream(mOut, 1024); - try { - switch (mSettings.getAuthType()) { - case CRAM_MD5: - if (hasCapability(CAPABILITY_AUTH_CRAM_MD5)) { - authCramMD5(); - } else { - throw new MessagingException( - "Server doesn't support encrypted passwords using CRAM-MD5."); - } - break; - - case PLAIN: - if (hasCapability(CAPABILITY_AUTH_PLAIN)) { - saslAuthPlain(); - } else if (!hasCapability(CAPABILITY_LOGINDISABLED)) { - login(); - } else { - throw new MessagingException( - "Server doesn't support unencrypted passwords using AUTH=PLAIN and LOGIN is disabled."); - } - break; - - default: + switch (mSettings.getAuthType()) { + case CRAM_MD5: + if (hasCapability(CAPABILITY_AUTH_CRAM_MD5)) { + authCramMD5(); + } else { throw new MessagingException( - "Unhandled authentication method found in the server settings (bug)."); + "Server doesn't support encrypted passwords using CRAM-MD5."); } - authSuccess = true; - } catch (ImapException ie) { - throw new AuthenticationFailedException(ie.getAlertText(), ie); + break; - } catch (MessagingException me) { - throw new AuthenticationFailedException(null, me); + case PLAIN: + if (hasCapability(CAPABILITY_AUTH_PLAIN)) { + saslAuthPlain(); + } else if (!hasCapability(CAPABILITY_LOGINDISABLED)) { + login(); + } else { + throw new MessagingException( + "Server doesn't support unencrypted passwords using AUTH=PLAIN and LOGIN is disabled."); + } + break; + + default: + throw new MessagingException( + "Unhandled authentication method found in the server settings (bug)."); } + authSuccess = true; if (K9.DEBUG) { Log.d(K9.LOG_TAG, CAPABILITY_COMPRESS_DEFLATE + " = " + hasCapability(CAPABILITY_COMPRESS_DEFLATE)); } diff --git a/src/com/fsck/k9/mail/store/Pop3Store.java b/src/com/fsck/k9/mail/store/Pop3Store.java index bdc0ef5bf..361f4258a 100644 --- a/src/com/fsck/k9/mail/store/Pop3Store.java +++ b/src/com/fsck/k9/mail/store/Pop3Store.java @@ -377,21 +377,23 @@ public class Pop3Store extends Store { } if (mAuthType == AuthType.CRAM_MD5) { + String b64Nonce = executeSimpleCommand("AUTH CRAM-MD5").replace("+ ", ""); + + String b64CRAM = Authentication.computeCramMd5(mUsername, mPassword, b64Nonce); try { - String b64Nonce = executeSimpleCommand("AUTH CRAM-MD5").replace("+ ", ""); - - String b64CRAM = Authentication.computeCramMd5(mUsername, mPassword, b64Nonce); executeSimpleCommand(b64CRAM); - - } catch (MessagingException me) { - throw new AuthenticationFailedException(null, me); + } catch (Pop3ErrorResponse e) { + throw new AuthenticationFailedException( + "POP3 CRAM-MD5 authentication failed: " + + e.getMessage(), e); } } else { + executeSimpleCommand(USER_COMMAND + " " + mUsername); try { - executeSimpleCommand(USER_COMMAND + " " + mUsername); executeSimpleCommand(PASS_COMMAND + " " + mPassword, true); - } catch (MessagingException me) { - throw new AuthenticationFailedException(null, me); + } catch (Pop3ErrorResponse e) { + throw new AuthenticationFailedException( + "POP3 login authentication failed: " + e.getMessage(), e); } } diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index d7478c89b..47ed37e36 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -714,7 +714,7 @@ public class SmtpTransport extends Transport { List respList = executeSimpleCommand("AUTH CRAM-MD5"); if (respList.size() != 1) { - throw new AuthenticationFailedException("Unable to negotiate CRAM-MD5"); + throw new MessagingException("Unable to negotiate CRAM-MD5"); } String b64Nonce = respList.get(0); @@ -722,8 +722,8 @@ public class SmtpTransport extends Transport { try { executeSimpleCommand(b64CRAMString, true); - } catch (MessagingException me) { - throw new AuthenticationFailedException("Unable to negotiate MD5 CRAM"); + } catch (NegativeSmtpReplyException exception) { + throw new AuthenticationFailedException(exception.getMessage(), exception); } } From 64fd04ece2a44815fe7e48c5a4017f0b658ef270 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Sat, 22 Feb 2014 17:51:18 -0500 Subject: [PATCH 06/25] POP3 authentication improvements Changes: Extract code and create login() and authCramMD5() methods. Implement the SASL PLAIN authentication mechanism. Its primary benefit is the explicit support for UTF-8. If the user has configured "PLAIN" authentication, then SASL PLAIN will be used, if available, otherwise login() will be used. Implement POP3 APOP authentication (issue 3218). If the user has configured "CRAM_MD5" authentication (a future commit will change this user option to a localized string "Encrypted password"), then SASL CRAM-MD5 will be used, if available, otherwise the availability of POP3 APOP will be checked and used (per RFC 2449, there is no APOP "capability"). Extend getCapabilities() to check for available authentication methods by sending the "AUTH" command with no arguments (http://tools.ietf.org/html/draft-myers-sasl-pop3-05). This never became a standard, but there are servers that support it, and Thunderbird includes this check. The SASL PLAIN and CRAM-MD5 authentication methods are not attempted unless the server professes to have the appropriate capability. (Previously, CRAM-MD5 was tried regardless of capability.) No check is made for the USER capability prior to use of that method. All this is the same behavior as in Thunderbird. Eliminate the testing for capabilities in cases where the test results are never used (PIPELINING, USER). Change when getCapabilities() is called. It is called once upon connection. If STARTTLS is negotiated (POP3 STLS), then getCapabilities() is called again after the connection is encrypted (and the server is authenticated), but before user authentication is attempted. --- src/com/fsck/k9/mail/filter/Hex.java | 4 +- src/com/fsck/k9/mail/store/Pop3Store.java | 173 +++++++++++++++++----- 2 files changed, 138 insertions(+), 39 deletions(-) diff --git a/src/com/fsck/k9/mail/filter/Hex.java b/src/com/fsck/k9/mail/filter/Hex.java index 92022203b..049af6e84 100644 --- a/src/com/fsck/k9/mail/filter/Hex.java +++ b/src/com/fsck/k9/mail/filter/Hex.java @@ -30,13 +30,13 @@ public class Hex { }; /** - * Converts an array of bytes into an array of characters representing the hexidecimal values of each byte in order. + * Converts an array of bytes into an array of characters representing the hexadecimal values of each byte in order. * The returned array will be double the length of the passed array, as it takes two characters to represent any * given byte. * * @param data * a byte[] to convert to Hex characters - * @return A char[] containing hexidecimal characters + * @return A char[] containing lower-case hexadecimal characters */ public static char[] encodeHex(byte[] data) { diff --git a/src/com/fsck/k9/mail/store/Pop3Store.java b/src/com/fsck/k9/mail/store/Pop3Store.java index 361f4258a..0751d287a 100644 --- a/src/com/fsck/k9/mail/store/Pop3Store.java +++ b/src/com/fsck/k9/mail/store/Pop3Store.java @@ -9,6 +9,8 @@ import com.fsck.k9.controller.MessageRetrievalListener; import com.fsck.k9.helper.Utility; import com.fsck.k9.mail.*; +import com.fsck.k9.mail.filter.Base64; +import com.fsck.k9.mail.filter.Hex; import com.fsck.k9.mail.internet.MimeMessage; import com.fsck.k9.net.ssl.TrustManagerFactory; import com.fsck.k9.net.ssl.TrustedSocketFactory; @@ -19,8 +21,11 @@ import javax.net.ssl.TrustManager; import java.io.*; import java.net.*; import java.security.GeneralSecurityException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; import java.util.LinkedList; import java.util.HashMap; @@ -47,6 +52,7 @@ public class Pop3Store extends Store { private static final String USER_COMMAND = "USER"; private static final String PASS_COMMAND = "PASS"; private static final String CAPA_COMMAND = "CAPA"; + private static final String AUTH_COMMAND = "AUTH"; private static final String STAT_COMMAND = "STAT"; private static final String LIST_COMMAND = "LIST"; private static final String UIDL_COMMAND = "UIDL"; @@ -57,9 +63,10 @@ public class Pop3Store extends Store { private static final String STLS_CAPABILITY = "STLS"; private static final String UIDL_CAPABILITY = "UIDL"; - private static final String PIPELINING_CAPABILITY = "PIPELINING"; - private static final String USER_CAPABILITY = "USER"; private static final String TOP_CAPABILITY = "TOP"; + private static final String SASL_CAPABILITY = "SASL"; + private static final String AUTH_PLAIN_CAPABILITY = "PLAIN"; + private static final String AUTH_CRAM_MD5_CAPABILITY = "CRAM-MD5"; /** * Decodes a Pop3Store URI. @@ -347,12 +354,11 @@ public class Pop3Store extends Store { throw new MessagingException("Unable to connect socket"); } - // Eat the banner - executeSimpleCommand(null); + String serverGreeting = executeSimpleCommand(null); + mCapabilities = getCapabilities(); if (mConnectionSecurity == CONNECTION_SECURITY_TLS_OPTIONAL || mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED) { - mCapabilities = getCapabilities(); if (mCapabilities.stls) { executeSimpleCommand(STLS_COMMAND); @@ -371,33 +377,33 @@ public class Pop3Store extends Store { if (!isOpen()) { throw new MessagingException("Unable to connect socket"); } + mCapabilities = getCapabilities(); } else if (mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED) { throw new MessagingException("TLS not supported but required"); } } - if (mAuthType == AuthType.CRAM_MD5) { - String b64Nonce = executeSimpleCommand("AUTH CRAM-MD5").replace("+ ", ""); + switch (mAuthType) { + case PLAIN: + if (mCapabilities.authPlain) { + authPlain(); + } else { + login(); + } + break; - String b64CRAM = Authentication.computeCramMd5(mUsername, mPassword, b64Nonce); - try { - executeSimpleCommand(b64CRAM); - } catch (Pop3ErrorResponse e) { - throw new AuthenticationFailedException( - "POP3 CRAM-MD5 authentication failed: " - + e.getMessage(), e); - } - } else { - executeSimpleCommand(USER_COMMAND + " " + mUsername); - try { - executeSimpleCommand(PASS_COMMAND + " " + mPassword, true); - } catch (Pop3ErrorResponse e) { - throw new AuthenticationFailedException( - "POP3 login authentication failed: " + e.getMessage(), e); + case CRAM_MD5: + if (mCapabilities.cramMD5) { + authCramMD5(); + } else { + authAPOP(serverGreeting); } + break; + + default: + throw new MessagingException( + "Unhandled authentication method found in the server settings (bug)."); } - - mCapabilities = getCapabilities(); } catch (SSLException e) { throw new CertificateValidationException(e.getMessage(), e); } catch (GeneralSecurityException gse) { @@ -416,6 +422,67 @@ public class Pop3Store extends Store { mUidToMsgNumMap.clear(); } + private void login() throws MessagingException { + executeSimpleCommand(USER_COMMAND + " " + mUsername); + try { + executeSimpleCommand(PASS_COMMAND + " " + mPassword, true); + } catch (Pop3ErrorResponse e) { + throw new AuthenticationFailedException( + "POP3 login authentication failed: " + e.getMessage(), e); + } + } + + private void authPlain() throws MessagingException { + executeSimpleCommand("AUTH PLAIN"); + try { + byte[] encodedBytes = Base64.encodeBase64(("\000" + mUsername + + "\000" + mPassword).getBytes()); + executeSimpleCommand(new String(encodedBytes), true); + } catch (Pop3ErrorResponse e) { + throw new AuthenticationFailedException( + "POP3 SASL auth PLAIN authentication failed: " + + e.getMessage(), e); + } + } + + private void authAPOP(String serverGreeting) throws MessagingException { + // regex based on RFC 2449 (3.) "Greeting" + String timestamp = serverGreeting.replaceFirst( + "^\\+OK *(?:\\[[^\\]]+\\])?[^<]*(<[^>]*>)?[^<]*$", "$1"); + if ("".equals(timestamp)) { + throw new MessagingException( + "APOP authentication is not supported"); + } + MessageDigest md; + try { + md = MessageDigest.getInstance("MD5"); + } catch (NoSuchAlgorithmException e) { + throw new MessagingException( + "MD5 failure during POP3 auth APOP", e); + } + byte[] digest = md.digest((timestamp + mPassword).getBytes()); + String hexDigest = new String(Hex.encodeHex(digest)); + try { + executeSimpleCommand("APOP " + mUsername + " " + hexDigest, true); + } catch (Pop3ErrorResponse e) { + throw new AuthenticationFailedException( + "POP3 APOP authentication failed: " + e.getMessage(), e); + } + } + + private void authCramMD5() throws MessagingException { + String b64Nonce = executeSimpleCommand("AUTH CRAM-MD5").replace("+ ", ""); + + String b64CRAM = Authentication.computeCramMd5(mUsername, mPassword, b64Nonce); + try { + executeSimpleCommand(b64CRAM, true); + } catch (Pop3ErrorResponse e) { + throw new AuthenticationFailedException( + "POP3 CRAM-MD5 authentication failed: " + + e.getMessage(), e); + } + } + @Override public boolean isOpen() { return (mIn != null && mOut != null && mSocket != null @@ -985,22 +1052,54 @@ public class Pop3Store extends Store { private Pop3Capabilities getCapabilities() throws IOException { Pop3Capabilities capabilities = new Pop3Capabilities(); + try { + /* + * Try sending an AUTH command with no arguments. + * + * The server may respond with a list of supported SASL + * authentication mechanisms. + * + * Ref.: http://tools.ietf.org/html/draft-myers-sasl-pop3-05 + * + * While this never became a standard, there are servers that + * support it, and Thunderbird includes this check. + */ + String response = executeSimpleCommand(AUTH_COMMAND); + while ((response = readLine()) != null) { + if (response.equals(".")) { + break; + } + response = response.toUpperCase(); + if (response.equals(AUTH_PLAIN_CAPABILITY)) { + capabilities.authPlain = true; + } else if (response.equals(AUTH_CRAM_MD5_CAPABILITY)) { + capabilities.cramMD5 = true; + } + } + } catch (MessagingException e) { + // Assume AUTH command with no arguments is not supported. + } try { String response = executeSimpleCommand(CAPA_COMMAND); while ((response = readLine()) != null) { if (response.equals(".")) { break; } - if (response.equalsIgnoreCase(STLS_CAPABILITY)) { + response = response.toUpperCase(); + if (response.equals(STLS_CAPABILITY)) { capabilities.stls = true; - } else if (response.equalsIgnoreCase(UIDL_CAPABILITY)) { + } else if (response.equals(UIDL_CAPABILITY)) { capabilities.uidl = true; - } else if (response.equalsIgnoreCase(PIPELINING_CAPABILITY)) { - capabilities.pipelining = true; - } else if (response.equalsIgnoreCase(USER_CAPABILITY)) { - capabilities.user = true; - } else if (response.equalsIgnoreCase(TOP_CAPABILITY)) { + } else if (response.equals(TOP_CAPABILITY)) { capabilities.top = true; + } else if (response.startsWith(SASL_CAPABILITY)) { + List saslAuthMechanisms = Arrays.asList(response.split(" ")); + if (saslAuthMechanisms.contains(AUTH_PLAIN_CAPABILITY)) { + capabilities.authPlain = true; + } + if (saslAuthMechanisms.contains(AUTH_CRAM_MD5_CAPABILITY)) { + capabilities.cramMD5 = true; + } } } @@ -1117,20 +1216,20 @@ public class Pop3Store extends Store { } static class Pop3Capabilities { + public boolean cramMD5; + public boolean authPlain; public boolean stls; public boolean top; - public boolean user; public boolean uidl; - public boolean pipelining; @Override public String toString() { - return String.format("STLS %b, TOP %b, USER %b, UIDL %b, PIPELINING %b", + return String.format("CRAM-MD5 %b, PLAIN %b, STLS %b, TOP %b, UIDL %b", + cramMD5, + authPlain, stls, top, - user, - uidl, - pipelining); + uidl); } } From f24ac67e4dec37b97311b9c376a73a08d1d49acd Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Mon, 3 Feb 2014 16:30:10 -0500 Subject: [PATCH 07/25] Ignore case for SMTP extension keywords The server is permitted to use mixed case keywords. This converts them to upper case on receipt. --- .../fsck/k9/mail/transport/SmtpTransport.java | 57 +++++++++---------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index 47ed37e36..4ae69df85 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -288,14 +288,14 @@ public class SmtpTransport extends Transport { } } - List results = sendHello(localHost); + HashMap extensions = sendHello(localHost); - m8bitEncodingAllowed = results.contains("8BITMIME"); + m8bitEncodingAllowed = extensions.containsKey("8BITMIME"); if (mConnectionSecurity == CONNECTION_SECURITY_TLS_OPTIONAL || mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED) { - if (results.contains("STARTTLS")) { + if (extensions.containsKey("STARTTLS")) { executeSimpleCommand("STARTTLS"); SSLContext sslContext = SSLContext.getInstance("TLS"); @@ -312,7 +312,7 @@ public class SmtpTransport extends Transport { * Now resend the EHLO. Required by RFC2487 Sec. 5.2, and more specifically, * Exim. */ - results = sendHello(localHost); + extensions = sendHello(localHost); } else if (mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED) { throw new MessagingException("TLS not supported but required"); } @@ -328,23 +328,18 @@ public class SmtpTransport extends Transport { boolean authLoginSupported = false; boolean authPlainSupported = false; boolean authCramMD5Supported = false; - for (String result : results) { - if (result.matches(".*AUTH.*LOGIN.*$")) { - authLoginSupported = true; - } - if (result.matches(".*AUTH.*PLAIN.*$")) { - authPlainSupported = true; - } - if (result.matches(".*AUTH.*CRAM-MD5.*$")) { - authCramMD5Supported = true; - } - if (result.matches(".*SIZE \\d*$")) { - try { - mLargestAcceptableMessage = Integer.parseInt(result.substring(result.lastIndexOf(' ') + 1)); - } catch (Exception e) { - if (K9.DEBUG && K9.DEBUG_PROTOCOL_SMTP) { - Log.d(K9.LOG_TAG, "Tried to parse " + result + " and get an int out of the last word", e); - } + if (extensions.containsKey("AUTH")) { + List saslMech = Arrays.asList(extensions.get("AUTH").split(" ")); + authLoginSupported = saslMech.contains("LOGIN"); + authPlainSupported = saslMech.contains("PLAIN"); + authCramMD5Supported = saslMech.contains("CRAM-MD5"); + } + if (extensions.containsKey("SIZE")) { + try { + mLargestAcceptableMessage = Integer.parseInt(extensions.get("SIZE")); + } catch (Exception e) { + if (K9.DEBUG && K9.DEBUG_PROTOCOL_SMTP) { + Log.d(K9.LOG_TAG, "Tried to parse " + extensions.get("SIZE") + " and get an int", e); } } } @@ -410,19 +405,24 @@ public class SmtpTransport extends Transport { * @param host * The EHLO/HELO parameter as defined by the RFC. * - * @return The list of capabilities as returned by the EHLO command or an empty list. + * @return A (possibly empty) {@code HashMap} of extensions (upper case) and + * their parameters (possibly 0 length) as returned by the EHLO command * * @throws IOException * In case of a network error. * @throws MessagingException * In case of a malformed response. */ - private List sendHello(String host) throws IOException, MessagingException { + private HashMap sendHello(String host) throws IOException, MessagingException { + HashMap extensions = new HashMap(); try { - //TODO: We currently assume the extension keywords returned by the server are always - // uppercased. But the RFC allows mixed-case keywords! - - return executeSimpleCommand("EHLO " + host); + List results = executeSimpleCommand("EHLO " + host); + // Remove the EHLO greeting response + results.remove(0); + for (String result : results) { + String[] pair = result.split(" ", 2); + extensions.put(pair[0].toUpperCase(), pair.length == 1 ? "" : pair[1]); + } } catch (NegativeSmtpReplyException e) { if (K9.DEBUG) { Log.v(K9.LOG_TAG, "Server doesn't support the EHLO command. Trying HELO..."); @@ -434,8 +434,7 @@ public class SmtpTransport extends Transport { Log.w(K9.LOG_TAG, "Server doesn't support the HELO command. Continuing anyway."); } } - - return new ArrayList(0); + return extensions; } @Override From dc9720ca13aaab7fa7f6196f376c495b704df2d2 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Thu, 13 Feb 2014 17:18:16 -0500 Subject: [PATCH 08/25] Use localized strings for authentication type AUTOMATIC = "Automatic" PLAIN = "Normal password" CRAM_MD5 = "Encrypted password" SMTP also uses LOGIN. No localized text was associated with that because a future commit will remove that option. (The text is similar to that of Thunderbird's) --- res/values/strings.xml | 4 +++ .../activity/setup/AccountSetupIncoming.java | 35 ++++++------------- .../activity/setup/AccountSetupOutgoing.java | 27 ++++---------- src/com/fsck/k9/mail/AuthType.java | 32 +++++++++++++++++ src/com/fsck/k9/mail/ServerSettings.java | 6 ++-- src/com/fsck/k9/mail/store/ImapStore.java | 25 +++++-------- src/com/fsck/k9/mail/store/Pop3Store.java | 20 +++-------- src/com/fsck/k9/mail/store/WebDavStore.java | 2 +- .../fsck/k9/mail/transport/SmtpTransport.java | 35 ++++++------------- .../k9/mail/transport/imap/ImapSettings.java | 2 +- .../fsck/k9/preferences/SettingsExporter.java | 4 +-- .../fsck/k9/preferences/SettingsImporter.java | 6 ++-- .../fsck/k9/mail/store/ImapStoreUriTest.java | 20 ++++++----- 13 files changed, 99 insertions(+), 119 deletions(-) create mode 100644 src/com/fsck/k9/mail/AuthType.java diff --git a/res/values/strings.xml b/res/values/strings.xml index 98ab1e85b..8e592c38a 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -385,6 +385,10 @@ Please submit bug reports, contribute new features and ask questions at IMAP Exchange (WebDAV) + Automatic + Normal password + Encrypted password + Incoming server settings Username Password diff --git a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java index e58fc8846..cdb91eea9 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java @@ -18,6 +18,7 @@ import com.fsck.k9.*; import com.fsck.k9.activity.K9Activity; import com.fsck.k9.activity.setup.AccountSetupCheckSettings.CheckDirection; import com.fsck.k9.helper.Utility; +import com.fsck.k9.mail.AuthType; import com.fsck.k9.mail.ConnectionSecurity; import com.fsck.k9.mail.ServerSettings; import com.fsck.k9.mail.Store; @@ -58,11 +59,6 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener ConnectionSecurity.STARTTLS_REQUIRED }; - private static final String[] AUTH_TYPES = { - "PLAIN", "CRAM_MD5" - }; - - private int[] mAccountPorts; private String mStoreType; private EditText mUsernameView; @@ -83,6 +79,7 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener private CheckBox mCompressionWifi; private CheckBox mCompressionOther; private CheckBox mSubscribedFoldersOnly; + private ArrayAdapter mAuthTypeAdapter; public static void actionIncomingSettings(Activity context, Account account, boolean makeDefault) { Intent i = new Intent(context, AccountSetupIncoming.class); @@ -149,22 +146,16 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener new SpinnerOption(4, getString(R.string.account_setup_incoming_security_tls_label)), }; - // This needs to be kept in sync with the list at the top of the file. - // that makes me somewhat unhappy - SpinnerOption authTypeSpinnerOptions[] = { - new SpinnerOption(0, AUTH_TYPES[0]), - new SpinnerOption(1, AUTH_TYPES[1]) - }; - ArrayAdapter securityTypesAdapter = new ArrayAdapter(this, android.R.layout.simple_spinner_item, securityTypes); securityTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); mSecurityTypeView.setAdapter(securityTypesAdapter); - ArrayAdapter authTypesAdapter = new ArrayAdapter(this, - android.R.layout.simple_spinner_item, authTypeSpinnerOptions); - authTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); - mAuthTypeView.setAdapter(authTypesAdapter); + AuthType[] acceptableAuthTypes = {AuthType.PLAIN, AuthType.CRAM_MD5}; + mAuthTypeAdapter = new ArrayAdapter(this, + android.R.layout.simple_spinner_item, acceptableAuthTypes); + mAuthTypeAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); + mAuthTypeView.setAdapter(mAuthTypeAdapter); /* * Calls validateFields() which enables or disables the Next button @@ -217,13 +208,9 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener mPasswordView.setText(settings.password); } - if (settings.authenticationType != null) { - for (int i = 0; i < AUTH_TYPES.length; i++) { - if (AUTH_TYPES[i].equals(settings.authenticationType)) { - SpinnerOption.setSpinnerOptionValue(mAuthTypeView, i); - } - } - } + // The first item is selected if settings.authenticationType is null or is not in mAuthTypeAdapter + int position = mAuthTypeAdapter.getPosition(settings.authenticationType); + mAuthTypeView.setSelection(position, false); mStoreType = settings.type; if (Pop3Store.STORE_TYPE.equals(settings.type)) { @@ -407,7 +394,7 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener String username = mUsernameView.getText().toString(); String password = mPasswordView.getText().toString(); - String authType = ((SpinnerOption)mAuthTypeView.getSelectedItem()).label; + AuthType authType = (AuthType) mAuthTypeView.getSelectedItem(); String host = mServerView.getText().toString(); int port = Integer.parseInt(mPortView.getText().toString()); diff --git a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java index 6d1af8bc0..3b221b45f 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java @@ -17,7 +17,7 @@ import com.fsck.k9.*; import com.fsck.k9.activity.K9Activity; import com.fsck.k9.activity.setup.AccountSetupCheckSettings.CheckDirection; import com.fsck.k9.helper.Utility; -import com.fsck.k9.mail.transport.SmtpTransport; +import com.fsck.k9.mail.AuthType; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -48,13 +48,6 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, "webdav", "webdav+ssl", "webdav+ssl+", "webdav+tls", "webdav+tls+" }; */ - private static final String authTypes[] = { - SmtpTransport.AUTH_AUTOMATIC, - SmtpTransport.AUTH_LOGIN, - SmtpTransport.AUTH_PLAIN, - SmtpTransport.AUTH_CRAM_MD5, - }; - private EditText mUsernameView; private EditText mPasswordView; private EditText mServerView; @@ -127,18 +120,13 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, new SpinnerOption(4, getString(R.string.account_setup_incoming_security_tls_label)), }; - SpinnerOption authTypeSpinnerOptions[] = new SpinnerOption[authTypes.length]; - for (int i = 0; i < authTypes.length; i++) { - authTypeSpinnerOptions[i] = new SpinnerOption(i, authTypes[i]); - } - ArrayAdapter securityTypesAdapter = new ArrayAdapter(this, android.R.layout.simple_spinner_item, securityTypes); securityTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); mSecurityTypeView.setAdapter(securityTypesAdapter); - ArrayAdapter authTypesAdapter = new ArrayAdapter(this, - android.R.layout.simple_spinner_item, authTypeSpinnerOptions); + ArrayAdapter authTypesAdapter = new ArrayAdapter(this, + android.R.layout.simple_spinner_item, AuthType.values()); authTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); mAuthTypeView.setAdapter(authTypesAdapter); @@ -208,11 +196,8 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, } if (authType != null) { - for (int i = 0; i < authTypes.length; i++) { - if (authTypes[i].equals(authType)) { - SpinnerOption.setSpinnerOptionValue(mAuthTypeView, i); - } - } + int position = AuthType.valueOf(authType).ordinal(); + mAuthTypeView.setSelection(position, false); } // Select currently configured security type @@ -305,7 +290,7 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, String passwordEnc = URLEncoder.encode(mPasswordView.getText().toString(), "UTF-8"); String userInfo = null; - String authType = ((SpinnerOption)mAuthTypeView.getSelectedItem()).label; + String authType = ((AuthType) mAuthTypeView.getSelectedItem()).name(); if (mRequireLoginView.isChecked()) { userInfo = usernameEnc + ":" + passwordEnc + ":" + authType; } diff --git a/src/com/fsck/k9/mail/AuthType.java b/src/com/fsck/k9/mail/AuthType.java new file mode 100644 index 000000000..9c2425e3c --- /dev/null +++ b/src/com/fsck/k9/mail/AuthType.java @@ -0,0 +1,32 @@ +package com.fsck.k9.mail; + +import com.fsck.k9.K9; +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. + */ + 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), + LOGIN(0); + + private final int mResourceId; + + private AuthType(int id) { + mResourceId = id; + } + + @Override + public String toString() { + if (mResourceId == 0) { + return name(); + } else { + return K9.app.getString(mResourceId); + } + } +} diff --git a/src/com/fsck/k9/mail/ServerSettings.java b/src/com/fsck/k9/mail/ServerSettings.java index 9ba43556e..50207863d 100644 --- a/src/com/fsck/k9/mail/ServerSettings.java +++ b/src/com/fsck/k9/mail/ServerSettings.java @@ -48,7 +48,7 @@ public class ServerSettings { * * {@code null} if not applicable for the store or transport. */ - public final String authenticationType; + public final AuthType authenticationType; /** * The username part of the credentials needed to authenticate to the server. @@ -91,7 +91,7 @@ public class ServerSettings { * see {@link ServerSettings#password} */ public ServerSettings(String type, String host, int port, - ConnectionSecurity connectionSecurity, String authenticationType, String username, + ConnectionSecurity connectionSecurity, AuthType authenticationType, String username, String password) { this.type = type; this.host = host; @@ -124,7 +124,7 @@ public class ServerSettings { * see {@link ServerSettings#extra} */ public ServerSettings(String type, String host, int port, - ConnectionSecurity connectionSecurity, String authenticationType, String username, + ConnectionSecurity connectionSecurity, AuthType authenticationType, String username, String password, Map extra) { this.type = type; this.host = host; diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index f557a852e..a30023421 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -69,6 +69,7 @@ import com.fsck.k9.helper.StringUtils; import com.fsck.k9.helper.Utility; import com.fsck.k9.helper.power.TracingPowerManager; import com.fsck.k9.helper.power.TracingPowerManager.TracingWakeLock; +import com.fsck.k9.mail.AuthType; import com.fsck.k9.mail.Authentication; import com.fsck.k9.mail.AuthenticationFailedException; import com.fsck.k9.mail.Body; @@ -117,8 +118,6 @@ public class ImapStore extends Store { public static final int CONNECTION_SECURITY_SSL_REQUIRED = 3; public static final int CONNECTION_SECURITY_SSL_OPTIONAL = 4; - public enum AuthType { PLAIN, CRAM_MD5 } - private static final int IDLE_READ_TIMEOUT_INCREMENT = 5 * 60 * 1000; private static final int IDLE_FAILURE_COUNT_LIMIT = 10; private static int MAX_DELAY_TIME = 5 * 60 * 1000; // 5 minutes @@ -163,7 +162,7 @@ public class ImapStore extends Store { String host; int port; ConnectionSecurity connectionSecurity; - String authenticationType = null; + AuthType authenticationType = null; String username = null; String password = null; String pathPrefix = null; @@ -209,14 +208,14 @@ public class ImapStore extends Store { if (userinfo.endsWith(":")) { // Password is empty. This can only happen after an account was imported. - authenticationType = AuthType.valueOf(userInfoParts[0]).name(); + authenticationType = AuthType.valueOf(userInfoParts[0]); username = URLDecoder.decode(userInfoParts[1], "UTF-8"); } else if (userInfoParts.length == 2) { - authenticationType = AuthType.PLAIN.name(); + authenticationType = AuthType.PLAIN; username = URLDecoder.decode(userInfoParts[0], "UTF-8"); password = URLDecoder.decode(userInfoParts[1], "UTF-8"); } else { - authenticationType = AuthType.valueOf(userInfoParts[0]).name(); + authenticationType = AuthType.valueOf(userInfoParts[0]); username = URLDecoder.decode(userInfoParts[1], "UTF-8"); password = URLDecoder.decode(userInfoParts[2], "UTF-8"); } @@ -291,15 +290,9 @@ public class ImapStore extends Store { break; } - AuthType authType; - try { - authType = AuthType.valueOf(server.authenticationType); - } catch (Exception e) { - throw new IllegalArgumentException("Invalid authentication type: " + - server.authenticationType); - } + AuthType authType = server.authenticationType; - String userInfo = authType.toString() + ":" + userEnc + ":" + passwordEnc; + String userInfo = authType.name() + ":" + userEnc + ":" + passwordEnc; try { Map extra = server.getExtra(); String path = null; @@ -334,7 +327,7 @@ public class ImapStore extends Store { public final String pathPrefix; protected ImapStoreSettings(String host, int port, ConnectionSecurity connectionSecurity, - String authenticationType, String username, String password, + AuthType authenticationType, String username, String password, boolean autodetectNamespace, String pathPrefix) { super(STORE_TYPE, host, port, connectionSecurity, authenticationType, username, password); @@ -485,7 +478,7 @@ public class ImapStore extends Store { break; } - mAuthType = AuthType.valueOf(settings.authenticationType); + mAuthType = settings.authenticationType; mUsername = settings.username; mPassword = settings.password; diff --git a/src/com/fsck/k9/mail/store/Pop3Store.java b/src/com/fsck/k9/mail/store/Pop3Store.java index 0751d287a..9a4f9fb9e 100644 --- a/src/com/fsck/k9/mail/store/Pop3Store.java +++ b/src/com/fsck/k9/mail/store/Pop3Store.java @@ -43,11 +43,6 @@ public class Pop3Store extends Store { public static final int CONNECTION_SECURITY_SSL_REQUIRED = 3; public static final int CONNECTION_SECURITY_SSL_OPTIONAL = 4; - private enum AuthType { - PLAIN, - CRAM_MD5 - } - private static final String STLS_COMMAND = "STLS"; private static final String USER_COMMAND = "USER"; private static final String PASS_COMMAND = "PASS"; @@ -120,7 +115,7 @@ public class Pop3Store extends Store { port = pop3Uri.getPort(); } - String authType = AuthType.PLAIN.name(); + AuthType authType = AuthType.PLAIN; if (pop3Uri.getUserInfo() != null) { try { int userIndex = 0, passwordIndex = 1; @@ -131,7 +126,7 @@ public class Pop3Store extends Store { // after an account was imported (so authType and username are present). userIndex++; passwordIndex++; - authType = userInfoParts[0]; + authType = AuthType.valueOf(userInfoParts[0]); } username = URLDecoder.decode(userInfoParts[userIndex], "UTF-8"); if (userInfoParts.length > passwordIndex) { @@ -190,14 +185,7 @@ public class Pop3Store extends Store { break; } - try { - AuthType.valueOf(server.authenticationType); - } catch (Exception e) { - throw new IllegalArgumentException("Invalid authentication type (" + - server.authenticationType + ")"); - } - - String userInfo = server.authenticationType + ":" + userEnc + ":" + passwordEnc; + String userInfo = server.authenticationType.name() + ":" + userEnc + ":" + passwordEnc; try { return new URI(scheme, userInfo, server.host, server.port, null, null, null).toString(); @@ -257,7 +245,7 @@ public class Pop3Store extends Store { mUsername = settings.username; mPassword = settings.password; - mAuthType = AuthType.valueOf(settings.authenticationType); + mAuthType = settings.authenticationType; } @Override diff --git a/src/com/fsck/k9/mail/store/WebDavStore.java b/src/com/fsck/k9/mail/store/WebDavStore.java index 01b45655a..df352cd2b 100644 --- a/src/com/fsck/k9/mail/store/WebDavStore.java +++ b/src/com/fsck/k9/mail/store/WebDavStore.java @@ -270,7 +270,7 @@ public class WebDavStore extends Store { public final String mailboxPath; protected WebDavStoreSettings(String host, int port, ConnectionSecurity connectionSecurity, - String authenticationType, String username, String password, String alias, + AuthType authenticationType, String username, String password, String alias, String path, String authPath, String mailboxPath) { super(STORE_TYPE, host, port, connectionSecurity, authenticationType, username, password); diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index 4ae69df85..3e029d954 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -39,15 +39,6 @@ public class SmtpTransport extends Transport { public static final int CONNECTION_SECURITY_SSL_REQUIRED = 3; public static final int CONNECTION_SECURITY_SSL_OPTIONAL = 4; - public static final String AUTH_PLAIN = "PLAIN"; - - public static final String AUTH_CRAM_MD5 = "CRAM_MD5"; - - public static final String AUTH_LOGIN = "LOGIN"; - - public static final String AUTH_AUTOMATIC = "AUTOMATIC"; - - /** * Decodes a SmtpTransport URI. * @@ -64,7 +55,7 @@ public class SmtpTransport extends Transport { String host; int port; ConnectionSecurity connectionSecurity; - String authenticationType = AUTH_AUTOMATIC; + AuthType authType = AuthType.AUTOMATIC; String username = null; String password = null; @@ -109,7 +100,7 @@ public class SmtpTransport extends Transport { password = URLDecoder.decode(userInfoParts[1], "UTF-8"); } if (userInfoParts.length > 2) { - authenticationType = userInfoParts[2]; + authType = AuthType.valueOf(userInfoParts[2]); } } catch (UnsupportedEncodingException enc) { // This shouldn't happen since the encoding is hardcoded to UTF-8 @@ -118,7 +109,7 @@ public class SmtpTransport extends Transport { } return new ServerSettings(TRANSPORT_TYPE, host, port, connectionSecurity, - authenticationType, username, password); + authType, username, password); } /** @@ -165,15 +156,11 @@ public class SmtpTransport extends Transport { break; } - String authType = server.authenticationType; - if (!(AUTH_AUTOMATIC.equals(authType) || - AUTH_LOGIN.equals(authType) || - AUTH_PLAIN.equals(authType) || - AUTH_CRAM_MD5.equals(authType))) { - throw new IllegalArgumentException("Invalid authentication type: " + authType); + String userInfo = userEnc + ":" + passwordEnc; + AuthType authType = server.authenticationType; + if (authType != null) { + userInfo += ":" + authType.name(); } - - String userInfo = userEnc + ":" + passwordEnc + ":" + authType; try { return new URI(scheme, userInfo, server.host, server.port, null, null, null).toString(); @@ -187,7 +174,7 @@ public class SmtpTransport extends Transport { int mPort; String mUsername; String mPassword; - String mAuthType; + AuthType mAuthType; int mConnectionSecurity; Socket mSocket; PeekableInputStream mIn; @@ -318,9 +305,9 @@ public class SmtpTransport extends Transport { } } - boolean useAuthLogin = AUTH_LOGIN.equals(mAuthType); - boolean useAuthPlain = AUTH_PLAIN.equals(mAuthType); - boolean useAuthCramMD5 = AUTH_CRAM_MD5.equals(mAuthType); + boolean useAuthLogin = AuthType.LOGIN.equals(mAuthType); + boolean useAuthPlain = AuthType.PLAIN.equals(mAuthType); + boolean useAuthCramMD5 = AuthType.CRAM_MD5.equals(mAuthType); // Automatically choose best authentication method if none was explicitly selected boolean useAutomaticAuth = !(useAuthLogin || useAuthPlain || useAuthCramMD5); diff --git a/src/com/fsck/k9/mail/transport/imap/ImapSettings.java b/src/com/fsck/k9/mail/transport/imap/ImapSettings.java index 63c22f6d9..b68c42f89 100644 --- a/src/com/fsck/k9/mail/transport/imap/ImapSettings.java +++ b/src/com/fsck/k9/mail/transport/imap/ImapSettings.java @@ -1,7 +1,7 @@ package com.fsck.k9.mail.transport.imap; +import com.fsck.k9.mail.AuthType; import com.fsck.k9.mail.store.ImapStore; -import com.fsck.k9.mail.store.ImapStore.AuthType; import com.fsck.k9.mail.store.ImapStore.ImapConnection; /** diff --git a/src/com/fsck/k9/preferences/SettingsExporter.java b/src/com/fsck/k9/preferences/SettingsExporter.java index 2497376aa..b09ef359d 100644 --- a/src/com/fsck/k9/preferences/SettingsExporter.java +++ b/src/com/fsck/k9/preferences/SettingsExporter.java @@ -230,7 +230,7 @@ public class SettingsExporter { writeElement(serializer, PORT_ELEMENT, Integer.toString(incoming.port)); } writeElement(serializer, CONNECTION_SECURITY_ELEMENT, incoming.connectionSecurity.name()); - writeElement(serializer, AUTHENTICATION_TYPE_ELEMENT, incoming.authenticationType); + writeElement(serializer, AUTHENTICATION_TYPE_ELEMENT, incoming.authenticationType.name()); writeElement(serializer, USERNAME_ELEMENT, incoming.username); // XXX For now we don't export the password //writeElement(serializer, PASSWORD_ELEMENT, incoming.password); @@ -257,7 +257,7 @@ public class SettingsExporter { writeElement(serializer, PORT_ELEMENT, Integer.toString(outgoing.port)); } writeElement(serializer, CONNECTION_SECURITY_ELEMENT, outgoing.connectionSecurity.name()); - writeElement(serializer, AUTHENTICATION_TYPE_ELEMENT, outgoing.authenticationType); + writeElement(serializer, AUTHENTICATION_TYPE_ELEMENT, outgoing.authenticationType.name()); writeElement(serializer, USERNAME_ELEMENT, outgoing.username); // XXX For now we don't export the password //writeElement(serializer, PASSWORD_ELEMENT, outgoing.password); diff --git a/src/com/fsck/k9/preferences/SettingsImporter.java b/src/com/fsck/k9/preferences/SettingsImporter.java index c9fd6557c..9a77614b0 100644 --- a/src/com/fsck/k9/preferences/SettingsImporter.java +++ b/src/com/fsck/k9/preferences/SettingsImporter.java @@ -23,6 +23,7 @@ import com.fsck.k9.Identity; import com.fsck.k9.K9; import com.fsck.k9.Preferences; import com.fsck.k9.helper.Utility; +import com.fsck.k9.mail.AuthType; import com.fsck.k9.mail.ConnectionSecurity; import com.fsck.k9.mail.ServerSettings; import com.fsck.k9.mail.Store; @@ -971,7 +972,8 @@ public class SettingsImporter { } else if (SettingsExporter.CONNECTION_SECURITY_ELEMENT.equals(element)) { server.connectionSecurity = getText(xpp); } else if (SettingsExporter.AUTHENTICATION_TYPE_ELEMENT.equals(element)) { - server.authenticationType = getText(xpp); + String text = getText(xpp); + server.authenticationType = AuthType.valueOf(text); } else if (SettingsExporter.USERNAME_ELEMENT.equals(element)) { server.username = getText(xpp); } else if (SettingsExporter.PASSWORD_ELEMENT.equals(element)) { @@ -1140,7 +1142,7 @@ public class SettingsImporter { public String host; public String port; public String connectionSecurity; - public String authenticationType; + public AuthType authenticationType; public String username; public String password; public ImportedSettings extras; diff --git a/tests/src/com/fsck/k9/mail/store/ImapStoreUriTest.java b/tests/src/com/fsck/k9/mail/store/ImapStoreUriTest.java index 4f5f7129e..442b443ce 100644 --- a/tests/src/com/fsck/k9/mail/store/ImapStoreUriTest.java +++ b/tests/src/com/fsck/k9/mail/store/ImapStoreUriTest.java @@ -2,6 +2,8 @@ package com.fsck.k9.mail.store; import java.util.HashMap; import java.util.Map; + +import com.fsck.k9.mail.AuthType; import com.fsck.k9.mail.ConnectionSecurity; import com.fsck.k9.mail.ServerSettings; import com.fsck.k9.mail.Store; @@ -13,7 +15,7 @@ public class ImapStoreUriTest extends TestCase { String uri = "imap://PLAIN:user:pass@server:143/0%7CcustomPathPrefix"; ServerSettings settings = Store.decodeStoreUri(uri); - assertEquals("PLAIN", settings.authenticationType); + assertEquals(AuthType.PLAIN, settings.authenticationType); assertEquals("user", settings.username); assertEquals("pass", settings.password); assertEquals("server", settings.host); @@ -26,7 +28,7 @@ public class ImapStoreUriTest extends TestCase { String uri = "imap://PLAIN:user:pass@server:143/"; ServerSettings settings = Store.decodeStoreUri(uri); - assertEquals("PLAIN", settings.authenticationType); + assertEquals(AuthType.PLAIN, settings.authenticationType); assertEquals("user", settings.username); assertEquals("pass", settings.password); assertEquals("server", settings.host); @@ -38,7 +40,7 @@ public class ImapStoreUriTest extends TestCase { String uri = "imap://PLAIN:user:pass@server:143/customPathPrefix"; ServerSettings settings = Store.decodeStoreUri(uri); - assertEquals("PLAIN", settings.authenticationType); + assertEquals(AuthType.PLAIN, settings.authenticationType); assertEquals("user", settings.username); assertEquals("pass", settings.password); assertEquals("server", settings.host); @@ -51,7 +53,7 @@ public class ImapStoreUriTest extends TestCase { String uri = "imap://PLAIN:user:pass@server:143/0%7C"; ServerSettings settings = Store.decodeStoreUri(uri); - assertEquals("PLAIN", settings.authenticationType); + assertEquals(AuthType.PLAIN, settings.authenticationType); assertEquals("user", settings.username); assertEquals("pass", settings.password); assertEquals("server", settings.host); @@ -64,7 +66,7 @@ public class ImapStoreUriTest extends TestCase { String uri = "imap://PLAIN:user:pass@server:143/1%7CcustomPathPrefix"; ServerSettings settings = Store.decodeStoreUri(uri); - assertEquals("PLAIN", settings.authenticationType); + assertEquals(AuthType.PLAIN, settings.authenticationType); assertEquals("user", settings.username); assertEquals("pass", settings.password); assertEquals("server", settings.host); @@ -80,7 +82,7 @@ public class ImapStoreUriTest extends TestCase { extra.put("pathPrefix", "customPathPrefix"); ServerSettings settings = new ServerSettings(ImapStore.STORE_TYPE, "server", 143, - ConnectionSecurity.NONE, "PLAIN", "user", "pass", extra); + ConnectionSecurity.NONE, AuthType.PLAIN, "user", "pass", extra); String uri = Store.createStoreUri(settings); @@ -93,7 +95,7 @@ public class ImapStoreUriTest extends TestCase { extra.put("pathPrefix", ""); ServerSettings settings = new ServerSettings(ImapStore.STORE_TYPE, "server", 143, - ConnectionSecurity.NONE, "PLAIN", "user", "pass", extra); + ConnectionSecurity.NONE, AuthType.PLAIN, "user", "pass", extra); String uri = Store.createStoreUri(settings); @@ -102,7 +104,7 @@ public class ImapStoreUriTest extends TestCase { public void testCreateStoreUriImapNoExtra() { ServerSettings settings = new ServerSettings(ImapStore.STORE_TYPE, "server", 143, - ConnectionSecurity.NONE, "PLAIN", "user", "pass"); + ConnectionSecurity.NONE, AuthType.PLAIN, "user", "pass"); String uri = Store.createStoreUri(settings); @@ -114,7 +116,7 @@ public class ImapStoreUriTest extends TestCase { extra.put("autoDetectNamespace", "true"); ServerSettings settings = new ServerSettings(ImapStore.STORE_TYPE, "server", 143, - ConnectionSecurity.NONE, "PLAIN", "user", "pass", extra); + ConnectionSecurity.NONE, AuthType.PLAIN, "user", "pass", extra); String uri = Store.createStoreUri(settings); From 90fedf7125b2d644e57d3e8f6cd22d23fa075220 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Sat, 22 Feb 2014 18:52:20 -0500 Subject: [PATCH 09/25] Eliminate the LOGIN authentication option from SMTP The LOGIN option is no longer offered to users as a choice. This does *not* eliminate the SASL LOGIN authentication mechanism. Any pre-existing LOGIN setting or any imported LOGIN setting will still be recognized. In all cases, a user setting of either "Normal password" or "LOGIN" will result in the SASL PLAIN mechanism being tried first if available, otherwise SASL LOGIN will be tried if available. This mirrors similar behavior that exists for IMAP. --- .../fsck/k9/activity/setup/AccountSetupOutgoing.java | 12 +++++++----- src/com/fsck/k9/mail/transport/SmtpTransport.java | 11 +++-------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java index 3b221b45f..b90b1c0ab 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java @@ -56,6 +56,7 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, private ViewGroup mRequireLoginSettingsView; private Spinner mSecurityTypeView; private Spinner mAuthTypeView; + private ArrayAdapter mAuthTypeAdapter; private Button mNextButton; private Account mAccount; private boolean mMakeDefault; @@ -125,10 +126,11 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, securityTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); mSecurityTypeView.setAdapter(securityTypesAdapter); - ArrayAdapter authTypesAdapter = new ArrayAdapter(this, - android.R.layout.simple_spinner_item, AuthType.values()); - authTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); - mAuthTypeView.setAdapter(authTypesAdapter); + AuthType[] acceptableAuthTypes = {AuthType.AUTOMATIC, AuthType.PLAIN, AuthType.CRAM_MD5}; + mAuthTypeAdapter = new ArrayAdapter(this, + android.R.layout.simple_spinner_item, acceptableAuthTypes); + mAuthTypeAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); + mAuthTypeView.setAdapter(mAuthTypeAdapter); /* * Calls validateFields() which enables or disables the Next button @@ -196,7 +198,7 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, } if (authType != null) { - int position = AuthType.valueOf(authType).ordinal(); + int position = mAuthTypeAdapter.getPosition(AuthType.valueOf(authType)); mAuthTypeView.setSelection(position, false); } diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index 3e029d954..e0a48441f 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -305,12 +305,11 @@ public class SmtpTransport extends Transport { } } - boolean useAuthLogin = AuthType.LOGIN.equals(mAuthType); - boolean useAuthPlain = AuthType.PLAIN.equals(mAuthType); + 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 = !(useAuthLogin || useAuthPlain || useAuthCramMD5); + boolean useAutomaticAuth = !(useAuthPlain || useAuthCramMD5); boolean authLoginSupported = false; boolean authPlainSupported = false; @@ -360,11 +359,7 @@ public class SmtpTransport extends Transport { throw ex; } } - } else if (useAuthLogin || (useAutomaticAuth && authLoginSupported)) { - if (!authPlainSupported && K9.DEBUG && K9.DEBUG_PROTOCOL_SMTP) { - Log.d(K9.LOG_TAG, "Using LOGIN as authentication method although the " + - "server didn't advertise support for it in EHLO response."); - } + } else if (useAutomaticAuth && authLoginSupported) { saslAuthLogin(mUsername, mPassword); } else { throw new MessagingException("No valid authentication mechanism found."); From c7e46faf0a783e0d75d31a91ed0b213d8783ade9 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Thu, 13 Feb 2014 19:43:24 -0500 Subject: [PATCH 10/25] Simplify code with better use of enum ConnectionSecurity --- .../activity/setup/AccountSetupIncoming.java | 98 ++++++++++--------- .../activity/setup/AccountSetupOutgoing.java | 53 +++++----- src/com/fsck/k9/mail/ConnectionSecurity.java | 34 ++++--- src/com/fsck/k9/mail/store/ImapStore.java | 44 +++------ src/com/fsck/k9/mail/store/Pop3Store.java | 40 ++------ src/com/fsck/k9/mail/store/WebDavStore.java | 37 ++----- .../fsck/k9/mail/transport/SmtpTransport.java | 40 ++------ .../k9/mail/transport/imap/ImapSettings.java | 3 +- 8 files changed, 134 insertions(+), 215 deletions(-) diff --git a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java index cdb91eea9..73ab9327b 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java @@ -39,27 +39,13 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener private static final String EXTRA_ACCOUNT = "account"; private static final String EXTRA_MAKE_DEFAULT = "makeDefault"; - private static final int[] POP3_PORTS = { - 110, 995, 995, 110, 110 - }; + private static final String POP3_PORT = "110"; + private static final String POP3_SSL_PORT = "995"; + private static final String IMAP_PORT = "143"; + private static final String IMAP_SSL_PORT = "993"; + private static final String WEBDAV_PORT = "80"; + private static final String WEBDAV_SSL_PORT = "443"; - private static final int[] IMAP_PORTS = { - 143, 993, 993, 143, 143 - }; - - private static final int[] WEBDAV_PORTS = { - 80, 443, 443, 443, 443 - }; - - private static final ConnectionSecurity[] CONNECTION_SECURITY_TYPES = { - ConnectionSecurity.NONE, - ConnectionSecurity.SSL_TLS_OPTIONAL, - ConnectionSecurity.SSL_TLS_REQUIRED, - ConnectionSecurity.STARTTLS_OPTIONAL, - ConnectionSecurity.STARTTLS_REQUIRED - }; - - private int[] mAccountPorts; private String mStoreType; private EditText mUsernameView; private EditText mPasswordView; @@ -80,6 +66,8 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener private CheckBox mCompressionOther; private CheckBox mSubscribedFoldersOnly; private ArrayAdapter mAuthTypeAdapter; + private String mDefaultPort = ""; + private String mDefaultSslPort = ""; public static void actionIncomingSettings(Activity context, Account account, boolean makeDefault) { Intent i = new Intent(context, AccountSetupIncoming.class); @@ -136,18 +124,8 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener } }); - SpinnerOption securityTypes[] = { - new SpinnerOption(0, getString(R.string.account_setup_incoming_security_none_label)), - new SpinnerOption(1, - getString(R.string.account_setup_incoming_security_ssl_optional_label)), - new SpinnerOption(2, getString(R.string.account_setup_incoming_security_ssl_label)), - new SpinnerOption(3, - getString(R.string.account_setup_incoming_security_tls_optional_label)), - new SpinnerOption(4, getString(R.string.account_setup_incoming_security_tls_label)), - }; - - ArrayAdapter securityTypesAdapter = new ArrayAdapter(this, - android.R.layout.simple_spinner_item, securityTypes); + ArrayAdapter securityTypesAdapter = new ArrayAdapter(this, + android.R.layout.simple_spinner_item, ConnectionSecurity.values()); securityTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); mSecurityTypeView.setAdapter(securityTypesAdapter); @@ -212,10 +190,14 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener int position = mAuthTypeAdapter.getPosition(settings.authenticationType); mAuthTypeView.setSelection(position, false); + // Select currently configured security type + mSecurityTypeView.setSelection(settings.connectionSecurity.ordinal(), false); + mStoreType = settings.type; if (Pop3Store.STORE_TYPE.equals(settings.type)) { serverLabelView.setText(R.string.account_setup_incoming_pop_server_label); - mAccountPorts = POP3_PORTS; + mDefaultPort = POP3_PORT; + mDefaultSslPort = POP3_SSL_PORT; findViewById(R.id.imap_path_prefix_section).setVisibility(View.GONE); findViewById(R.id.webdav_advanced_header).setVisibility(View.GONE); findViewById(R.id.webdav_mailbox_alias_section).setVisibility(View.GONE); @@ -227,7 +209,8 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener mAccount.setDeletePolicy(Account.DELETE_POLICY_NEVER); } else if (ImapStore.STORE_TYPE.equals(settings.type)) { serverLabelView.setText(R.string.account_setup_incoming_imap_server_label); - mAccountPorts = IMAP_PORTS; + mDefaultPort = IMAP_PORT; + mDefaultSslPort = IMAP_SSL_PORT; ImapStoreSettings imapSettings = (ImapStoreSettings) settings; @@ -247,7 +230,8 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener } } else if (WebDavStore.STORE_TYPE.equals(settings.type)) { serverLabelView.setText(R.string.account_setup_incoming_webdav_server_label); - mAccountPorts = WEBDAV_PORTS; + mDefaultPort = WEBDAV_PORT; + mDefaultSslPort = WEBDAV_SSL_PORT; // Hide the unnecessary fields findViewById(R.id.imap_path_prefix_section).setVisibility(View.GONE); @@ -275,13 +259,6 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener throw new Exception("Unknown account type: " + mAccount.getStoreUri()); } - // Select currently configured security type - for (int i = 0; i < CONNECTION_SECURITY_TYPES.length; i++) { - if (CONNECTION_SECURITY_TYPES[i] == settings.connectionSecurity) { - SpinnerOption.setSpinnerOptionValue(mSecurityTypeView, i); - } - } - /* * Updates the port when the user changes the security type. This allows * us to show a reasonable default which the user can change. @@ -340,10 +317,38 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener } private void updatePortFromSecurityType() { - if (mAccountPorts != null) { - int securityType = (Integer)((SpinnerOption)mSecurityTypeView.getSelectedItem()).value; - mPortView.setText(Integer.toString(mAccountPorts[securityType])); + ConnectionSecurity securityType = (ConnectionSecurity) mSecurityTypeView.getSelectedItem(); + mPortView.setText(getDefaultPort(securityType)); + } + + private String getDefaultPort(ConnectionSecurity securityType) { + String port; + switch (securityType) { + case NONE: + port = mDefaultPort; + break; + case STARTTLS_OPTIONAL: + case STARTTLS_REQUIRED: + if (WebDavStore.STORE_TYPE.equals(mStoreType)) { + /* + * The concept of STARTTLS is not really applicable for WebDav and should never + * have been made a user-selectable option. But now we must support the setting + * if it exists. + */ + port = mDefaultSslPort; + } else { + port = mDefaultPort; + } + break; + case SSL_TLS_OPTIONAL: + case SSL_TLS_REQUIRED: + port = mDefaultSslPort; + break; + default: + Log.e(K9.LOG_TAG, "Unhandled ConnectionSecurity type encountered"); + port = ""; } + return port; } @Override @@ -389,8 +394,7 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener protected void onNext() { try { - ConnectionSecurity connectionSecurity = CONNECTION_SECURITY_TYPES[ - (Integer)((SpinnerOption)mSecurityTypeView.getSelectedItem()).value]; + ConnectionSecurity connectionSecurity = (ConnectionSecurity) mSecurityTypeView.getSelectedItem(); String username = mUsernameView.getText().toString(); String password = mPasswordView.getText().toString(); diff --git a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java index b90b1c0ab..032828979 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java @@ -18,6 +18,7 @@ import com.fsck.k9.activity.K9Activity; import com.fsck.k9.activity.setup.AccountSetupCheckSettings.CheckDirection; import com.fsck.k9.helper.Utility; import com.fsck.k9.mail.AuthType; +import com.fsck.k9.mail.ConnectionSecurity; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -31,23 +32,12 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, private static final String EXTRA_MAKE_DEFAULT = "makeDefault"; - private static final int smtpPorts[] = { - 587, 465, 465, 587, 587 - }; + private static final String SMTP_PORT = "587"; + private static final String SMTP_SSL_PORT = "465"; private static final String smtpSchemes[] = { "smtp", "smtp+ssl", "smtp+ssl+", "smtp+tls", "smtp+tls+" }; - /* - private static final int webdavPorts[] = - { - 80, 443, 443, 443, 443 - }; - private static final String webdavSchemes[] = - { - "webdav", "webdav+ssl", "webdav+ssl+", "webdav+tls", "webdav+tls+" - }; - */ private EditText mUsernameView; private EditText mPasswordView; private EditText mServerView; @@ -111,18 +101,8 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, mNextButton.setOnClickListener(this); mRequireLoginView.setOnCheckedChangeListener(this); - SpinnerOption securityTypes[] = { - new SpinnerOption(0, getString(R.string.account_setup_incoming_security_none_label)), - new SpinnerOption(1, - getString(R.string.account_setup_incoming_security_ssl_optional_label)), - new SpinnerOption(2, getString(R.string.account_setup_incoming_security_ssl_label)), - new SpinnerOption(3, - getString(R.string.account_setup_incoming_security_tls_optional_label)), - new SpinnerOption(4, getString(R.string.account_setup_incoming_security_tls_label)), - }; - - ArrayAdapter securityTypesAdapter = new ArrayAdapter(this, - android.R.layout.simple_spinner_item, securityTypes); + ArrayAdapter securityTypesAdapter = new ArrayAdapter(this, + android.R.layout.simple_spinner_item, ConnectionSecurity.values()); securityTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); mSecurityTypeView.setAdapter(securityTypesAdapter); @@ -267,8 +247,27 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, } private void updatePortFromSecurityType() { - int securityType = (Integer)((SpinnerOption)mSecurityTypeView.getSelectedItem()).value; - mPortView.setText(Integer.toString(smtpPorts[securityType])); + ConnectionSecurity securityType = (ConnectionSecurity) mSecurityTypeView.getSelectedItem(); + mPortView.setText(getDefaultSmtpPort(securityType)); + } + + private String getDefaultSmtpPort(ConnectionSecurity securityType) { + String port; + switch (securityType) { + case NONE: + case STARTTLS_OPTIONAL: + case STARTTLS_REQUIRED: + port = SMTP_PORT; + break; + case SSL_TLS_OPTIONAL: + case SSL_TLS_REQUIRED: + port = SMTP_SSL_PORT; + break; + default: + port = ""; + Log.e(K9.LOG_TAG, "Unhandled ConnectionSecurity type encountered"); + } + return port; } @Override diff --git a/src/com/fsck/k9/mail/ConnectionSecurity.java b/src/com/fsck/k9/mail/ConnectionSecurity.java index 98741303e..42ba3ee1a 100644 --- a/src/com/fsck/k9/mail/ConnectionSecurity.java +++ b/src/com/fsck/k9/mail/ConnectionSecurity.java @@ -1,19 +1,23 @@ package com.fsck.k9.mail; -/** - * The currently available connection security types. - * - *

- * Right now this enum is only used by {@link ServerSettings} and converted to store- or - * transport-specific constants in the different {@link Store} and {@link Transport} - * implementations. In the future we probably want to change this and use - * {@code ConnectionSecurity} exclusively. - *

- */ +import com.fsck.k9.K9; +import com.fsck.k9.R; + public enum ConnectionSecurity { - NONE, - STARTTLS_OPTIONAL, - STARTTLS_REQUIRED, - SSL_TLS_OPTIONAL, - SSL_TLS_REQUIRED + NONE(R.string.account_setup_incoming_security_none_label), + STARTTLS_OPTIONAL(R.string.account_setup_incoming_security_tls_optional_label), + STARTTLS_REQUIRED(R.string.account_setup_incoming_security_tls_label), + SSL_TLS_OPTIONAL(R.string.account_setup_incoming_security_ssl_optional_label), + SSL_TLS_REQUIRED(R.string.account_setup_incoming_security_ssl_label); + + private final int mResourceId; + + private ConnectionSecurity(int id) { + mResourceId = id; + } + + @Override + public String toString() { + return K9.app.getString(mResourceId); + } } diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index a30023421..4bd8fa12f 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -112,12 +112,6 @@ import com.jcraft.jzlib.ZOutputStream; public class ImapStore extends Store { public static final String STORE_TYPE = "IMAP"; - public static final int CONNECTION_SECURITY_NONE = 0; - public static final int CONNECTION_SECURITY_TLS_OPTIONAL = 1; - public static final int CONNECTION_SECURITY_TLS_REQUIRED = 2; - public static final int CONNECTION_SECURITY_SSL_REQUIRED = 3; - public static final int CONNECTION_SECURITY_SSL_OPTIONAL = 4; - private static final int IDLE_READ_TIMEOUT_INCREMENT = 5 * 60 * 1000; private static final int IDLE_FAILURE_COUNT_LIMIT = 10; private static int MAX_DELAY_TIME = 5 * 60 * 1000; // 5 minutes @@ -355,7 +349,7 @@ public class ImapStore extends Store { private int mPort; private String mUsername; private String mPassword; - private int mConnectionSecurity; + private ConnectionSecurity mConnectionSecurity; private AuthType mAuthType; private volatile String mPathPrefix; private volatile String mCombinedPrefix = null; @@ -374,7 +368,7 @@ public class ImapStore extends Store { } @Override - public int getConnectionSecurity() { + public ConnectionSecurity getConnectionSecurity() { return mConnectionSecurity; } @@ -460,23 +454,7 @@ public class ImapStore extends Store { mHost = settings.host; mPort = settings.port; - switch (settings.connectionSecurity) { - case NONE: - mConnectionSecurity = CONNECTION_SECURITY_NONE; - break; - case STARTTLS_OPTIONAL: - mConnectionSecurity = CONNECTION_SECURITY_TLS_OPTIONAL; - break; - case STARTTLS_REQUIRED: - mConnectionSecurity = CONNECTION_SECURITY_TLS_REQUIRED; - break; - case SSL_TLS_OPTIONAL: - mConnectionSecurity = CONNECTION_SECURITY_SSL_OPTIONAL; - break; - case SSL_TLS_REQUIRED: - mConnectionSecurity = CONNECTION_SECURITY_SSL_REQUIRED; - break; - } + mConnectionSecurity = settings.connectionSecurity; mAuthType = settings.authenticationType; mUsername = settings.username; @@ -2427,7 +2405,7 @@ public class ImapStore extends Store { } try { - int connectionSecurity = mSettings.getConnectionSecurity(); + ConnectionSecurity connectionSecurity = mSettings.getConnectionSecurity(); // Try all IPv4 and IPv6 addresses of the host InetAddress[] addresses = InetAddress.getAllByName(mSettings.getHost()); @@ -2441,10 +2419,10 @@ public class ImapStore extends Store { SocketAddress socketAddress = new InetSocketAddress(addresses[i], mSettings.getPort()); - if (connectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED || - connectionSecurity == CONNECTION_SECURITY_SSL_OPTIONAL) { + if (connectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED || + connectionSecurity == ConnectionSecurity.SSL_TLS_OPTIONAL) { SSLContext sslContext = SSLContext.getInstance("TLS"); - boolean secure = connectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED; + boolean secure = connectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED; sslContext .init(null, new TrustManager[] { TrustManagerFactory.get( @@ -2494,15 +2472,15 @@ public class ImapStore extends Store { } } - if (mSettings.getConnectionSecurity() == CONNECTION_SECURITY_TLS_OPTIONAL - || mSettings.getConnectionSecurity() == CONNECTION_SECURITY_TLS_REQUIRED) { + if (mSettings.getConnectionSecurity() == ConnectionSecurity.STARTTLS_OPTIONAL + || mSettings.getConnectionSecurity() == ConnectionSecurity.STARTTLS_REQUIRED) { if (hasCapability("STARTTLS")) { // STARTTLS executeSimpleCommand("STARTTLS"); SSLContext sslContext = SSLContext.getInstance("TLS"); - boolean secure = mSettings.getConnectionSecurity() == CONNECTION_SECURITY_TLS_REQUIRED; + boolean secure = mSettings.getConnectionSecurity() == ConnectionSecurity.STARTTLS_REQUIRED; sslContext.init(null, new TrustManager[] { TrustManagerFactory.get( mSettings.getHost(), @@ -2523,7 +2501,7 @@ public class ImapStore extends Store { if (responses.size() != 2) { throw new MessagingException("Invalid CAPABILITY response received"); } - } else if (mSettings.getConnectionSecurity() == CONNECTION_SECURITY_TLS_REQUIRED) { + } else if (mSettings.getConnectionSecurity() == ConnectionSecurity.STARTTLS_REQUIRED) { throw new MessagingException("TLS not supported but required"); } } diff --git a/src/com/fsck/k9/mail/store/Pop3Store.java b/src/com/fsck/k9/mail/store/Pop3Store.java index 9a4f9fb9e..59a53a8a0 100644 --- a/src/com/fsck/k9/mail/store/Pop3Store.java +++ b/src/com/fsck/k9/mail/store/Pop3Store.java @@ -37,12 +37,6 @@ import java.util.Set; public class Pop3Store extends Store { public static final String STORE_TYPE = "POP3"; - public static final int CONNECTION_SECURITY_NONE = 0; - public static final int CONNECTION_SECURITY_TLS_OPTIONAL = 1; - public static final int CONNECTION_SECURITY_TLS_REQUIRED = 2; - public static final int CONNECTION_SECURITY_SSL_REQUIRED = 3; - public static final int CONNECTION_SECURITY_SSL_OPTIONAL = 4; - private static final String STLS_COMMAND = "STLS"; private static final String USER_COMMAND = "USER"; private static final String PASS_COMMAND = "PASS"; @@ -200,7 +194,7 @@ public class Pop3Store extends Store { private String mUsername; private String mPassword; private AuthType mAuthType; - private int mConnectionSecurity; + private ConnectionSecurity mConnectionSecurity; private HashMap mFolders = new HashMap(); private Pop3Capabilities mCapabilities; @@ -225,23 +219,7 @@ public class Pop3Store extends Store { mHost = settings.host; mPort = settings.port; - switch (settings.connectionSecurity) { - case NONE: - mConnectionSecurity = CONNECTION_SECURITY_NONE; - break; - case STARTTLS_OPTIONAL: - mConnectionSecurity = CONNECTION_SECURITY_TLS_OPTIONAL; - break; - case STARTTLS_REQUIRED: - mConnectionSecurity = CONNECTION_SECURITY_TLS_REQUIRED; - break; - case SSL_TLS_OPTIONAL: - mConnectionSecurity = CONNECTION_SECURITY_SSL_OPTIONAL; - break; - case SSL_TLS_REQUIRED: - mConnectionSecurity = CONNECTION_SECURITY_SSL_REQUIRED; - break; - } + mConnectionSecurity = settings.connectionSecurity; mUsername = settings.username; mPassword = settings.password; @@ -321,10 +299,10 @@ public class Pop3Store extends Store { try { SocketAddress socketAddress = new InetSocketAddress(mHost, mPort); - if (mConnectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED || - mConnectionSecurity == CONNECTION_SECURITY_SSL_OPTIONAL) { + if (mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED || + mConnectionSecurity == ConnectionSecurity.SSL_TLS_OPTIONAL) { SSLContext sslContext = SSLContext.getInstance("TLS"); - final boolean secure = mConnectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED; + final boolean secure = mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED; sslContext.init(null, new TrustManager[] { TrustManagerFactory.get(mHost, mPort, secure) }, new SecureRandom()); @@ -345,14 +323,14 @@ public class Pop3Store extends Store { String serverGreeting = executeSimpleCommand(null); mCapabilities = getCapabilities(); - if (mConnectionSecurity == CONNECTION_SECURITY_TLS_OPTIONAL - || mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED) { + if (mConnectionSecurity == ConnectionSecurity.STARTTLS_OPTIONAL + || mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { if (mCapabilities.stls) { executeSimpleCommand(STLS_COMMAND); SSLContext sslContext = SSLContext.getInstance("TLS"); - boolean secure = mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED; + boolean secure = mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED; sslContext.init(null, new TrustManager[] { TrustManagerFactory.get( mHost, mPort, secure) }, @@ -366,7 +344,7 @@ public class Pop3Store extends Store { throw new MessagingException("Unable to connect socket"); } mCapabilities = getCapabilities(); - } else if (mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED) { + } else if (mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { throw new MessagingException("TLS not supported but required"); } } diff --git a/src/com/fsck/k9/mail/store/WebDavStore.java b/src/com/fsck/k9/mail/store/WebDavStore.java index df352cd2b..b0af15ef2 100644 --- a/src/com/fsck/k9/mail/store/WebDavStore.java +++ b/src/com/fsck/k9/mail/store/WebDavStore.java @@ -57,13 +57,6 @@ import java.util.zip.GZIPInputStream; public class WebDavStore extends Store { public static final String STORE_TYPE = "WebDAV"; - // Security options - private static final short CONNECTION_SECURITY_NONE = 0; - private static final short CONNECTION_SECURITY_TLS_OPTIONAL = 1; - private static final short CONNECTION_SECURITY_TLS_REQUIRED = 2; - private static final short CONNECTION_SECURITY_SSL_OPTIONAL = 3; - private static final short CONNECTION_SECURITY_SSL_REQUIRED = 4; - // Authentication types private static final short AUTH_TYPE_NONE = 0; private static final short AUTH_TYPE_BASIC = 1; @@ -298,7 +291,7 @@ public class WebDavStore extends Store { } - private short mConnectionSecurity; + private ConnectionSecurity mConnectionSecurity; private String mUsername; /* Stores the username for authentications */ private String mAlias; /* Stores the alias for the user's mailbox */ private String mPassword; /* Stores the password for authentications */ @@ -334,23 +327,7 @@ public class WebDavStore extends Store { mHost = settings.host; mPort = settings.port; - switch (settings.connectionSecurity) { - case NONE: - mConnectionSecurity = CONNECTION_SECURITY_NONE; - break; - case STARTTLS_OPTIONAL: - mConnectionSecurity = CONNECTION_SECURITY_TLS_OPTIONAL; - break; - case STARTTLS_REQUIRED: - mConnectionSecurity = CONNECTION_SECURITY_TLS_REQUIRED; - break; - case SSL_TLS_OPTIONAL: - mConnectionSecurity = CONNECTION_SECURITY_SSL_OPTIONAL; - break; - case SSL_TLS_REQUIRED: - mConnectionSecurity = CONNECTION_SECURITY_SSL_REQUIRED; - break; - } + mConnectionSecurity = settings.connectionSecurity; mUsername = settings.username; mPassword = settings.password; @@ -383,16 +360,16 @@ public class WebDavStore extends Store { // The inbox path would look like: "https://mail.domain.com/Exchange/alias/Inbox". mUrl = getRoot() + mPath + mMailboxPath; - mSecure = mConnectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED; + mSecure = mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED; mAuthString = "Basic " + Utility.base64Encode(mUsername + ":" + mPassword); } private String getRoot() { String root; - if (mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED || - mConnectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED || - mConnectionSecurity == CONNECTION_SECURITY_TLS_OPTIONAL || - mConnectionSecurity == CONNECTION_SECURITY_SSL_OPTIONAL) { + if (mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED || + mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED || + mConnectionSecurity == ConnectionSecurity.STARTTLS_OPTIONAL || + mConnectionSecurity == ConnectionSecurity.SSL_TLS_OPTIONAL) { root = "https"; } else { root = "http"; diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index e0a48441f..a52873595 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -33,12 +33,6 @@ import java.util.*; public class SmtpTransport extends Transport { public static final String TRANSPORT_TYPE = "SMTP"; - public static final int CONNECTION_SECURITY_NONE = 0; - public static final int CONNECTION_SECURITY_TLS_OPTIONAL = 1; - public static final int CONNECTION_SECURITY_TLS_REQUIRED = 2; - public static final int CONNECTION_SECURITY_SSL_REQUIRED = 3; - public static final int CONNECTION_SECURITY_SSL_OPTIONAL = 4; - /** * Decodes a SmtpTransport URI. * @@ -175,7 +169,7 @@ public class SmtpTransport extends Transport { String mUsername; String mPassword; AuthType mAuthType; - int mConnectionSecurity; + ConnectionSecurity mConnectionSecurity; Socket mSocket; PeekableInputStream mIn; OutputStream mOut; @@ -193,23 +187,7 @@ public class SmtpTransport extends Transport { mHost = settings.host; mPort = settings.port; - switch (settings.connectionSecurity) { - case NONE: - mConnectionSecurity = CONNECTION_SECURITY_NONE; - break; - case STARTTLS_OPTIONAL: - mConnectionSecurity = CONNECTION_SECURITY_TLS_OPTIONAL; - break; - case STARTTLS_REQUIRED: - mConnectionSecurity = CONNECTION_SECURITY_TLS_REQUIRED; - break; - case SSL_TLS_OPTIONAL: - mConnectionSecurity = CONNECTION_SECURITY_SSL_OPTIONAL; - break; - case SSL_TLS_REQUIRED: - mConnectionSecurity = CONNECTION_SECURITY_SSL_REQUIRED; - break; - } + mConnectionSecurity = settings.connectionSecurity; mAuthType = settings.authenticationType; mUsername = settings.username; @@ -223,10 +201,10 @@ public class SmtpTransport extends Transport { for (int i = 0; i < addresses.length; i++) { try { SocketAddress socketAddress = new InetSocketAddress(addresses[i], mPort); - if (mConnectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED || - mConnectionSecurity == CONNECTION_SECURITY_SSL_OPTIONAL) { + if (mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED || + mConnectionSecurity == ConnectionSecurity.SSL_TLS_OPTIONAL) { SSLContext sslContext = SSLContext.getInstance("TLS"); - boolean secure = mConnectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED; + boolean secure = mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED; sslContext.init(null, new TrustManager[] { TrustManagerFactory.get( mHost, mPort, secure) }, @@ -280,13 +258,13 @@ public class SmtpTransport extends Transport { m8bitEncodingAllowed = extensions.containsKey("8BITMIME"); - if (mConnectionSecurity == CONNECTION_SECURITY_TLS_OPTIONAL - || mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED) { + if (mConnectionSecurity == ConnectionSecurity.STARTTLS_OPTIONAL + || mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { if (extensions.containsKey("STARTTLS")) { executeSimpleCommand("STARTTLS"); SSLContext sslContext = SSLContext.getInstance("TLS"); - boolean secure = mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED; + boolean secure = mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED; sslContext.init(null, new TrustManager[] { TrustManagerFactory.get(mHost, mPort, secure) }, new SecureRandom()); @@ -300,7 +278,7 @@ public class SmtpTransport extends Transport { * Exim. */ extensions = sendHello(localHost); - } else if (mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED) { + } else if (mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { throw new MessagingException("TLS not supported but required"); } } diff --git a/src/com/fsck/k9/mail/transport/imap/ImapSettings.java b/src/com/fsck/k9/mail/transport/imap/ImapSettings.java index b68c42f89..dc765d54f 100644 --- a/src/com/fsck/k9/mail/transport/imap/ImapSettings.java +++ b/src/com/fsck/k9/mail/transport/imap/ImapSettings.java @@ -1,6 +1,7 @@ package com.fsck.k9.mail.transport.imap; import com.fsck.k9.mail.AuthType; +import com.fsck.k9.mail.ConnectionSecurity; import com.fsck.k9.mail.store.ImapStore; import com.fsck.k9.mail.store.ImapStore.ImapConnection; @@ -12,7 +13,7 @@ public interface ImapSettings { int getPort(); - int getConnectionSecurity(); + ConnectionSecurity getConnectionSecurity(); AuthType getAuthType(); From 23f8d53178fcfcf898bc3fc7ebc317c2f0afd4d2 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Sun, 9 Feb 2014 23:08:36 -0500 Subject: [PATCH 11/25] Use Transport.decodeTransportUri and Transport.createTransportUri rather than manually creating the URIs. This mirrors behavior that occurs in AccountSetupIncoming. --- .../setup/AccountSetupAccountType.java | 10 ++ .../activity/setup/AccountSetupOutgoing.java | 91 +++++++------------ 2 files changed, 41 insertions(+), 60 deletions(-) diff --git a/src/com/fsck/k9/activity/setup/AccountSetupAccountType.java b/src/com/fsck/k9/activity/setup/AccountSetupAccountType.java index f4a7f34cb..3e038166f 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupAccountType.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupAccountType.java @@ -55,6 +55,11 @@ public class AccountSetupAccountType extends K9Activity implements OnClickListen URI uri = new URI(mAccount.getStoreUri()); uri = new URI("pop3", uri.getUserInfo(), uri.getHost(), uri.getPort(), null, null, null); mAccount.setStoreUri(uri.toString()); + + uri = new URI(mAccount.getTransportUri()); + uri = new URI("smtp", uri.getUserInfo(), uri.getHost(), uri.getPort(), null, null, null); + mAccount.setTransportUri(uri.toString()); + AccountSetupIncoming.actionIncomingSettings(this, mAccount, mMakeDefault); finish(); } catch (Exception use) { @@ -68,6 +73,11 @@ public class AccountSetupAccountType extends K9Activity implements OnClickListen URI uri = new URI(mAccount.getStoreUri()); uri = new URI("imap", uri.getUserInfo(), uri.getHost(), uri.getPort(), null, null, null); mAccount.setStoreUri(uri.toString()); + + uri = new URI(mAccount.getTransportUri()); + uri = new URI("smtp", uri.getUserInfo(), uri.getHost(), uri.getPort(), null, null, null); + mAccount.setTransportUri(uri.toString()); + AccountSetupIncoming.actionIncomingSettings(this, mAccount, mMakeDefault); finish(); } catch (Exception use) { diff --git a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java index 032828979..d74e80088 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java @@ -19,12 +19,12 @@ import com.fsck.k9.activity.setup.AccountSetupCheckSettings.CheckDirection; import com.fsck.k9.helper.Utility; import com.fsck.k9.mail.AuthType; import com.fsck.k9.mail.ConnectionSecurity; +import com.fsck.k9.mail.ServerSettings; +import com.fsck.k9.mail.Transport; +import com.fsck.k9.mail.transport.SmtpTransport; -import java.io.UnsupportedEncodingException; import java.net.URI; import java.net.URISyntaxException; -import java.net.URLDecoder; -import java.net.URLEncoder; public class AccountSetupOutgoing extends K9Activity implements OnClickListener, OnCheckedChangeListener { @@ -35,9 +35,6 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, private static final String SMTP_PORT = "587"; private static final String SMTP_SSL_PORT = "465"; - private static final String smtpSchemes[] = { - "smtp", "smtp+ssl", "smtp+ssl+", "smtp+tls", "smtp+tls+" - }; private EditText mUsernameView; private EditText mPasswordView; private EditText mServerView; @@ -152,21 +149,9 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, } try { - URI uri = new URI(mAccount.getTransportUri()); - String username = null; - String password = null; - String authType = null; - if (uri.getUserInfo() != null) { - String[] userInfoParts = uri.getUserInfo().split(":"); - - username = URLDecoder.decode(userInfoParts[0], "UTF-8"); - if (userInfoParts.length > 1) { - password = URLDecoder.decode(userInfoParts[1], "UTF-8"); - } - if (userInfoParts.length > 2) { - authType = userInfoParts[2]; - } - } + ServerSettings settings = Transport.decodeTransportUri(mAccount.getTransportUri()); + String username = settings.username; + String password = settings.password; if (username != null) { mUsernameView.setText(username); @@ -177,17 +162,12 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, mPasswordView.setText(password); } - if (authType != null) { - int position = mAuthTypeAdapter.getPosition(AuthType.valueOf(authType)); - mAuthTypeView.setSelection(position, false); - } + // The first item is selected if settings.authenticationType is null or is not in mAuthTypeAdapter + int position = mAuthTypeAdapter.getPosition(settings.authenticationType); + mAuthTypeView.setSelection(position, false); // Select currently configured security type - for (int i = 0; i < smtpSchemes.length; i++) { - if (smtpSchemes[i].equals(uri.getScheme())) { - SpinnerOption.setSpinnerOptionValue(mSecurityTypeView, i); - } - } + mSecurityTypeView.setSelection(settings.connectionSecurity.ordinal(), false); /* * Updates the port when the user changes the security type. This allows @@ -209,12 +189,12 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, public void onNothingSelected(AdapterView parent) { /* unused */ } }); - if (uri.getHost() != null) { - mServerView.setText(uri.getHost()); + if (settings.host != null) { + mServerView.setText(settings.host); } - if (uri.getPort() != -1) { - mPortView.setText(Integer.toString(uri.getPort())); + if (settings.port != -1) { + mPortView.setText(Integer.toString(settings.port)); } else { updatePortFromSecurityType(); } @@ -284,34 +264,25 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, } protected void onNext() { - int securityType = (Integer)((SpinnerOption)mSecurityTypeView.getSelectedItem()).value; - URI uri; - try { - String usernameEnc = URLEncoder.encode(mUsernameView.getText().toString(), "UTF-8"); - String passwordEnc = URLEncoder.encode(mPasswordView.getText().toString(), "UTF-8"); - - String userInfo = null; - String authType = ((AuthType) mAuthTypeView.getSelectedItem()).name(); - if (mRequireLoginView.isChecked()) { - userInfo = usernameEnc + ":" + passwordEnc + ":" + authType; - } - String newHost = mServerView.getText().toString(); - int newPort = Integer.parseInt(mPortView.getText().toString()); - uri = new URI(smtpSchemes[securityType], userInfo, newHost, newPort, null, null, null); - mAccount.deleteCertificate(newHost, newPort, CheckDirection.OUTGOING); - mAccount.setTransportUri(uri.toString()); - AccountSetupCheckSettings.actionCheckSettings(this, mAccount, CheckDirection.OUTGOING); - } catch (UnsupportedEncodingException enc) { - // This really shouldn't happen since the encoding is hardcoded to UTF-8 - Log.e(K9.LOG_TAG, "Couldn't urlencode username or password.", enc); - } catch (Exception e) { - /* - * It's unrecoverable if we cannot create a URI from components that - * we validated to be safe. - */ - failure(e); + ConnectionSecurity securityType = (ConnectionSecurity) mSecurityTypeView.getSelectedItem(); + String uri; + String username = null; + String password = null; + AuthType authType = null; + if (mRequireLoginView.isChecked()) { + username = mUsernameView.getText().toString(); + password = mPasswordView.getText().toString(); + authType = (AuthType) mAuthTypeView.getSelectedItem(); } + String newHost = mServerView.getText().toString(); + int newPort = Integer.parseInt(mPortView.getText().toString()); + String type = SmtpTransport.TRANSPORT_TYPE; + ServerSettings server = new ServerSettings(type, newHost, newPort, securityType, authType, username, password); + uri = Transport.createTransportUri(server); + mAccount.deleteCertificate(newHost, newPort, CheckDirection.OUTGOING); + mAccount.setTransportUri(uri); + AccountSetupCheckSettings.actionCheckSettings(this, mAccount, CheckDirection.OUTGOING); } public void onClick(View v) { From f7d397ea0913bb3a22bf44377a850fc8b5ab7f4d Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Mon, 10 Feb 2014 16:38:13 -0500 Subject: [PATCH 12/25] 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) { From 540de158a07d47ee60b2481b53e052684e792237 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Fri, 14 Feb 2014 10:37:44 -0500 Subject: [PATCH 13/25] Change the PLAIN auth. option text based on encryption If the user chooses a connection security option which assures the use of encryption, then the PLAIN auth. option is labeled "Normal password", otherwise it is labeled "Password, transmitted insecurely". This is similar to Thunderbird's behavior. --- res/values/strings.xml | 1 + .../activity/setup/AccountSetupIncoming.java | 19 ++++++-- .../activity/setup/AccountSetupOutgoing.java | 19 ++++++-- src/com/fsck/k9/mail/AuthType.java | 43 +++++++++++++++++-- 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/res/values/strings.xml b/res/values/strings.xml index 9de519bd3..5a1c8103c 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -386,6 +386,7 @@ Please submit bug reports, contribute new features and ask questions at Exchange (WebDAV) Normal password + Password, transmitted insecurely Encrypted password Incoming server settings diff --git a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java index 73ab9327b..f05376499 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java @@ -129,10 +129,7 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener securityTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); mSecurityTypeView.setAdapter(securityTypesAdapter); - AuthType[] acceptableAuthTypes = {AuthType.PLAIN, AuthType.CRAM_MD5}; - mAuthTypeAdapter = new ArrayAdapter(this, - android.R.layout.simple_spinner_item, acceptableAuthTypes); - mAuthTypeAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); + mAuthTypeAdapter = AuthType.getArrayAdapter(this); mAuthTypeView.setAdapter(mAuthTypeAdapter); /* @@ -186,6 +183,8 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener mPasswordView.setText(settings.password); } + updateAuthPlainTextFromSecurityType(settings.connectionSecurity); + // The first item is selected if settings.authenticationType is null or is not in mAuthTypeAdapter int position = mAuthTypeAdapter.getPosition(settings.authenticationType); mAuthTypeView.setSelection(position, false); @@ -319,6 +318,7 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener private void updatePortFromSecurityType() { ConnectionSecurity securityType = (ConnectionSecurity) mSecurityTypeView.getSelectedItem(); mPortView.setText(getDefaultPort(securityType)); + updateAuthPlainTextFromSecurityType(securityType); } private String getDefaultPort(ConnectionSecurity securityType) { @@ -351,6 +351,17 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener return port; } + private void updateAuthPlainTextFromSecurityType(ConnectionSecurity securityType) { + switch (securityType) { + case NONE: + case STARTTLS_OPTIONAL: + AuthType.PLAIN.useInsecureText(true, mAuthTypeAdapter); + break; + default: + AuthType.PLAIN.useInsecureText(false, mAuthTypeAdapter); + } + } + @Override public void onActivityResult(int requestCode, int resultCode, Intent data) { if (resultCode == RESULT_OK) { diff --git a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java index d74e80088..bb77aef11 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java @@ -103,10 +103,7 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, securityTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); mSecurityTypeView.setAdapter(securityTypesAdapter); - AuthType[] acceptableAuthTypes = {AuthType.AUTOMATIC, AuthType.PLAIN, AuthType.CRAM_MD5}; - mAuthTypeAdapter = new ArrayAdapter(this, - android.R.layout.simple_spinner_item, acceptableAuthTypes); - mAuthTypeAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); + mAuthTypeAdapter = AuthType.getArrayAdapter(this); mAuthTypeView.setAdapter(mAuthTypeAdapter); /* @@ -162,6 +159,8 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, mPasswordView.setText(password); } + updateAuthPlainTextFromSecurityType(settings.connectionSecurity); + // The first item is selected if settings.authenticationType is null or is not in mAuthTypeAdapter int position = mAuthTypeAdapter.getPosition(settings.authenticationType); mAuthTypeView.setSelection(position, false); @@ -229,6 +228,7 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, private void updatePortFromSecurityType() { ConnectionSecurity securityType = (ConnectionSecurity) mSecurityTypeView.getSelectedItem(); mPortView.setText(getDefaultSmtpPort(securityType)); + updateAuthPlainTextFromSecurityType(securityType); } private String getDefaultSmtpPort(ConnectionSecurity securityType) { @@ -250,6 +250,17 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, return port; } + private void updateAuthPlainTextFromSecurityType(ConnectionSecurity securityType) { + switch (securityType) { + case NONE: + case STARTTLS_OPTIONAL: + AuthType.PLAIN.useInsecureText(true, mAuthTypeAdapter); + break; + default: + AuthType.PLAIN.useInsecureText(false, mAuthTypeAdapter); + } + } + @Override public void onActivityResult(int requestCode, int resultCode, Intent data) { if (resultCode == RESULT_OK) { diff --git a/src/com/fsck/k9/mail/AuthType.java b/src/com/fsck/k9/mail/AuthType.java index c77371661..52ffe5866 100644 --- a/src/com/fsck/k9/mail/AuthType.java +++ b/src/com/fsck/k9/mail/AuthType.java @@ -1,10 +1,11 @@ package com.fsck.k9.mail; +import android.content.Context; +import android.widget.ArrayAdapter; import com.fsck.k9.K9; import com.fsck.k9.R; public enum AuthType { - /* * 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 @@ -16,7 +17,20 @@ public enum AuthType { * their original names have been retained for backward compatibility with * user settings. */ - PLAIN(R.string.account_setup_auth_type_normal_password), + + PLAIN(R.string.account_setup_auth_type_normal_password){ + + @Override + public void useInsecureText(boolean insecure, ArrayAdapter authTypesAdapter) { + if (insecure) { + mResourceId = R.string.account_setup_auth_type_insecure_password; + } else { + mResourceId = R.string.account_setup_auth_type_normal_password; + } + authTypesAdapter.notifyDataSetChanged(); + } + }, + CRAM_MD5(R.string.account_setup_auth_type_encrypted_password), /* @@ -29,12 +43,35 @@ public enum AuthType { LOGIN(0); - private final int mResourceId; + static public ArrayAdapter getArrayAdapter(Context context) { + AuthType[] authTypes = {PLAIN, CRAM_MD5}; + ArrayAdapter authTypesAdapter = new ArrayAdapter(context, + android.R.layout.simple_spinner_item, authTypes); + authTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); + return authTypesAdapter; + } + + int mResourceId; private AuthType(int id) { mResourceId = id; } + /** + * Used to select an appropriate localized text label for the + * {@code AuthType.PLAIN} option presented to users. + * + * @param insecure + *

+ * A value of {@code true} will use "Normal password". + *

+ * A value of {@code false} will use + * "Password, transmitted insecurely" + */ + public void useInsecureText(boolean insecure, ArrayAdapter authTypesAdapter) { + // Do nothing. Overridden in AuthType.PLAIN + } + @Override public String toString() { if (mResourceId == 0) { From 0509e1541cbd841d52e74b883dc3e27fefc8c5a3 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Mon, 3 Mar 2014 10:08:07 -0500 Subject: [PATCH 14/25] Use Locale.US where appropriate --- src/com/fsck/k9/mail/store/ImapStore.java | 4 ++-- src/com/fsck/k9/mail/store/Pop3Store.java | 4 ++-- src/com/fsck/k9/mail/transport/SmtpTransport.java | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index 4bd8fa12f..e34d47b67 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -2656,13 +2656,13 @@ public class ImapStore extends Store { String tag; byte[] username = mSettings.getUsername().getBytes(); byte[] password = mSettings.getPassword().getBytes(); - tag = sendCommand(String.format("LOGIN {%d%s}", + tag = sendCommand(String.format(Locale.US, "LOGIN {%d%s}", username.length, hasLiteralPlus ? "+" : ""), true); if (!hasLiteralPlus) { readContinuationResponse(tag); } mOut.write(username); - mOut.write(String.format(" {%d%s}\r\n", password.length, + mOut.write(String.format(Locale.US, " {%d%s}\r\n", password.length, hasLiteralPlus ? "+" : "").getBytes()); if (!hasLiteralPlus) { mOut.flush(); diff --git a/src/com/fsck/k9/mail/store/Pop3Store.java b/src/com/fsck/k9/mail/store/Pop3Store.java index 59a53a8a0..8acc7e079 100644 --- a/src/com/fsck/k9/mail/store/Pop3Store.java +++ b/src/com/fsck/k9/mail/store/Pop3Store.java @@ -1035,7 +1035,7 @@ public class Pop3Store extends Store { if (response.equals(".")) { break; } - response = response.toUpperCase(); + response = response.toUpperCase(Locale.US); if (response.equals(AUTH_PLAIN_CAPABILITY)) { capabilities.authPlain = true; } else if (response.equals(AUTH_CRAM_MD5_CAPABILITY)) { @@ -1051,7 +1051,7 @@ public class Pop3Store extends Store { if (response.equals(".")) { break; } - response = response.toUpperCase(); + response = response.toUpperCase(Locale.US); if (response.equals(STLS_CAPABILITY)) { capabilities.stls = true; } else if (response.equals(UIDL_CAPABILITY)) { diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index 4c5101ff0..9c8142b4c 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -410,7 +410,7 @@ public class SmtpTransport extends Transport { results.remove(0); for (String result : results) { String[] pair = result.split(" ", 2); - extensions.put(pair[0].toUpperCase(), pair.length == 1 ? "" : pair[1]); + extensions.put(pair[0].toUpperCase(Locale.US), pair.length == 1 ? "" : pair[1]); } } catch (NegativeSmtpReplyException e) { if (K9.DEBUG) { From 39590d49bd383065ef6cdb954d61c3724835a97c Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Thu, 27 Feb 2014 12:50:05 -0500 Subject: [PATCH 15/25] Notify user of certificate errors while pushing Previously, if a server's certificate failed authentication while connecting for push (if, for example, the certificate had expired), then the attempt to connect would fail, and another attempt would be tried later. After a certain number of failed attempts, no further attempts would be made. Meanwhile, the user is oblivious to the failures, and it could be quite some time before the user realizes that they are not getting email. Even when they do realize it, they would not know the cause. With this commit, users receive a notification when such failures occur while connecting for push. (These notifications are already generated with failures while polling.) Tapping the notification will take the user to the relevant server settings where they can choose to accept the certificate. --- src/com/fsck/k9/controller/MessagingController.java | 4 ++-- src/com/fsck/k9/mail/store/ImapStore.java | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/com/fsck/k9/controller/MessagingController.java b/src/com/fsck/k9/controller/MessagingController.java index 63d2afa63..d1fdf20c0 100644 --- a/src/com/fsck/k9/controller/MessagingController.java +++ b/src/com/fsck/k9/controller/MessagingController.java @@ -2627,7 +2627,7 @@ public class MessagingController implements Runnable { } } - private void notifyUserIfCertificateProblem(Context context, Exception e, + public static void notifyUserIfCertificateProblem(Context context, Exception e, Account account, boolean incoming) { if (!(e instanceof CertificateValidationException)) { return; @@ -5037,7 +5037,7 @@ public class MessagingController implements Runnable { * @param ringAndVibrate * {@code true}, if ringtone/vibration are allowed. {@code false}, otherwise. */ - private void configureNotification(NotificationCompat.Builder builder, String ringtone, + private static void configureNotification(NotificationCompat.Builder builder, String ringtone, long[] vibrationPattern, Integer ledColor, int ledSpeed, boolean ringAndVibrate) { // if it's quiet time, then we shouldn't be ringing, buzzing or flashing diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index e34d47b67..62b22b604 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -65,6 +65,7 @@ import com.fsck.k9.Account; import com.fsck.k9.K9; import com.fsck.k9.R; import com.fsck.k9.controller.MessageRetrievalListener; +import com.fsck.k9.controller.MessagingController; import com.fsck.k9.helper.StringUtils; import com.fsck.k9.helper.Utility; import com.fsck.k9.helper.power.TracingPowerManager; @@ -3138,6 +3139,7 @@ public class ImapStore extends Store { if (stop.get()) { Log.i(K9.LOG_TAG, "Got exception while idling, but stop is set for " + getLogId()); } else { + MessagingController.notifyUserIfCertificateProblem(K9.app, e, getAccount(), true); receiver.pushError("Push error for " + getName(), e); Log.e(K9.LOG_TAG, "Got exception while idling for " + getLogId(), e); int delayTimeInt = delayTime.get(); From daea7f1ecdb4515298a6c57dd5a829689426c2c9 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Wed, 26 Feb 2014 16:50:21 -0500 Subject: [PATCH 16/25] Eliminate the 'if available' connection security options These options originated in the AOSP email client from which K-9 Mail was forked. They provide an odd combination of 2 features: 1. Don't bother to authenticate the server's certificate (applies to both SSL/TLS and STARTTLS); i.e., blindly accept all certificates. This is generally a bad security policy which is susceptible to MITM attacks. 2. If STARTTLS is selected but the server doesn't claim to support STARTTLS, then proceed without using encryption. This, too, is a bad security policy which is susceptible to MITM attacks. Since the time that K-9 Mail was forked, a couple things have changed: > K-9 Mail has implemented the ability for users to review and permanently accept individual certificates that would otherwise fail authentication. With this ability, there is no need for a user to subject themselves to the ongoing risks of feature 1. above. Hence, this commit removes feature 1. > The AOSP email client has changed its behavior and no longer permits a security downgrade to an unencrypted connection if the server doesn't claim to support STARTTLS (i.e., they eliminated feature 2. above). K-9 Mail should do the same. It's unlikely that a server is going to provide STARTTLS on an intermittent basis, so providing a contingency for such unusual behavior is an unnecessary risk. Hence, this commit removes that feature as well. Effect on existing users: If the old connection security setting was "SSL/TLS (if available)" (which now gets remapped to "SSL/TLS"), and the server does not provide a certificate that can be authenticated, then a "Certificate error for " notification is generated telling the user to check their server settings. Tapping the notification takes the user to the relevant server settings, where the user can tap "Next" to review the certificate and choose to permanently accept it. This process would occur during the first syncing of folders after application upgrade or (in the case of SMTP) during the first attempt to send a message. If the connection security setting was "STARTTLS (if available)" (which now gets remapped to "STARTTLS"), and the server does not provide a certificate that can be authenticated, then the same process as above would occur. If the old connection security setting was "STARTTLS (if available)", and the server doesn't claim to support STARTTLS, then the user would get a certificate error notification which would lead them to the server's settings. There they would need to choose a different connection security -- most likely "NONE". If they didn't change anything but instead just tapped "Next", the server settings would be checked again and a dialog would pop up saying, "Cannot connect to server. (STARTTLS connection security not available)". (The implementation of notifications when STARTTLS is not available is not actually included here -- it's in the commit that follows.) Regarding the changes to providers.xml: in cases where the scheme ended with "+ssl", the schemes were simply updated by appending "+". In cases where the scheme ended with "+tls", a check of the server was made to assure that STARTTLS was available before appending "+" to the scheme. Domains paran.com and nate.com failed the check and were removed because no current information could be found. Domains me.com and mac.com also failed and were updated based on http://support.apple.com/kb/ht4864. --- res/values-ca/strings.xml | 4 -- res/values-cs/strings.xml | 4 -- res/values-da/strings.xml | 4 -- res/values-de/strings.xml | 4 -- res/values-el/strings.xml | 4 -- res/values-es/strings.xml | 4 -- res/values-fi/strings.xml | 4 -- res/values-fr/strings.xml | 4 -- res/values-gl/strings.xml | 4 -- res/values-hu/strings.xml | 4 -- res/values-it/strings.xml | 4 -- res/values-iw/strings.xml | 4 -- res/values-ja/strings.xml | 4 -- res/values-ko/strings.xml | 4 -- res/values-lt/strings.xml | 4 -- res/values-nl/strings.xml | 4 -- res/values-pl/strings.xml | 4 -- res/values-pt-rBR/strings.xml | 4 -- res/values-ru/strings.xml | 4 -- res/values-sk/strings.xml | 4 -- res/values-sv/strings.xml | 4 -- res/values-tr/strings.xml | 4 -- res/values-uk/strings.xml | 4 -- res/values-zh-rCN/strings.xml | 4 -- res/values-zh-rTW/strings.xml | 4 -- res/values/strings.xml | 6 +-- res/xml/providers.xml | 53 +++++++------------ .../activity/setup/AccountSetupIncoming.java | 3 -- .../activity/setup/AccountSetupOutgoing.java | 3 -- src/com/fsck/k9/mail/ConnectionSecurity.java | 2 - src/com/fsck/k9/mail/store/ImapStore.java | 50 ++++++++--------- src/com/fsck/k9/mail/store/Pop3Store.java | 50 ++++++++--------- src/com/fsck/k9/mail/store/WebDavStore.java | 38 +++++++------ .../fsck/k9/mail/transport/SmtpTransport.java | 50 ++++++++--------- 34 files changed, 104 insertions(+), 251 deletions(-) diff --git a/res/values-ca/strings.xml b/res/values-ca/strings.xml index 2759c3c97..d6c88a688 100644 --- a/res/values-ca/strings.xml +++ b/res/values-ca/strings.xml @@ -389,10 +389,6 @@ Si us plau, envia\'ns els errors, contribueix a millorar-lo a Tipus de seguretat Tipus d\'autenticació Cap - SSL/TLS (si és disponible) - SSL/TLS (sempre) - STARTTLS (si és disponible) - STARTTLS (sempre) Quan esborro missatges No els esborris del servidor diff --git a/res/values-cs/strings.xml b/res/values-cs/strings.xml index c28891705..7e83382d1 100644 --- a/res/values-cs/strings.xml +++ b/res/values-cs/strings.xml @@ -393,10 +393,6 @@ Posílejte prosím chybová hlášení, přispívejte novými funkcemi a ptejte Typ zabezpečení Typ ověření Žádné - SSL/TLS (je-li dostupné) - SSL/TLS (vždy) - STARTTLS (je-li dostupné) - STARTTLS (vždy) Když smažu zprávu Nemazat ji na serveru diff --git a/res/values-da/strings.xml b/res/values-da/strings.xml index c39b56048..bece6f525 100644 --- a/res/values-da/strings.xml +++ b/res/values-da/strings.xml @@ -389,10 +389,6 @@ Vær venlig at sende fejlrapporter, anmodning om nye funktioner, og spørgsmål Sikkerhed Autentifikations type Ingen - SSL/TLS (hvis tilgængelig) - SSL/TLS (altid) - STARTTLS (hvis tilgængelig) - STARTTLS (altid) Når jeg sletter en mail Undlad at slette på server diff --git a/res/values-de/strings.xml b/res/values-de/strings.xml index 7df1011f0..3e685f573 100644 --- a/res/values-de/strings.xml +++ b/res/values-de/strings.xml @@ -387,10 +387,6 @@ Um Fehler zu melden, neue Funktionen vorzuschlagen oder Fragen zu stellen, besuc Sicherheitstyp Authentifizierungstyp Keine Verschlüsselung - SSL/TLS (falls verfügbar) - SSL/TLS (immer) - STARTTLS (falls verfügbar) - STARTTLS (immer) Beim Löschen von Nachrichten Nie von Server löschen diff --git a/res/values-el/strings.xml b/res/values-el/strings.xml index 9852d9964..7dde8f171 100644 --- a/res/values-el/strings.xml +++ b/res/values-el/strings.xml @@ -388,10 +388,6 @@ Ασφάλεια Πιστοποίηση Καμιά - SSL/TLS (αν υπάρχει) - SSL/TLS (πάντοτε) - STARTTLS (αν υπάρχει) - STARTTLS (πάντοτε) Κατά τη διαγραφή μηνύματος Να μη διαγράφεται στο server diff --git a/res/values-es/strings.xml b/res/values-es/strings.xml index b20ddf6e8..abb27998d 100644 --- a/res/values-es/strings.xml +++ b/res/values-es/strings.xml @@ -388,10 +388,6 @@ Por favor, envía los errores detectados, contribuye con nuevas funcionalidades Tipo de Seguridad Tipo de autentificación Ninguna - SSL/TLS (si disponible) - SSL/TLS (siempre) - STARTTLS (si disponible) - STARTTLS (siempre) Borrado de mensajes No borrar del servidor diff --git a/res/values-fi/strings.xml b/res/values-fi/strings.xml index 7cdc8bdf4..59e6817e8 100755 --- a/res/values-fi/strings.xml +++ b/res/values-fi/strings.xml @@ -389,10 +389,6 @@ Virheraportit, osallistuminen projektiin ja kysymykset: Mene osoitteeseen Suojauksen tyyppi Todennuksen tyyppi Ei mitään - SSL/TLS (jos käytettävissä) - SSL/TLS (aina) - STARTTLS (jos käytettävissä) - STARTTLS (aina) Kun viesti poistetaan Älä poista palvelimelta diff --git a/res/values-fr/strings.xml b/res/values-fr/strings.xml index 01a545959..e925198f4 100644 --- a/res/values-fr/strings.xml +++ b/res/values-fr/strings.xml @@ -414,10 +414,6 @@ de plus Type de sécurité Type d\'authentification Aucun - SSL/TLS (si disponible) - SSL/TLS (toujours) - STARTTLS (si disponible) - STARTTLS (toujours) Supprimer les messages du serveur\u00A0: Jamais diff --git a/res/values-gl/strings.xml b/res/values-gl/strings.xml index a891ec599..81ea6942a 100644 --- a/res/values-gl/strings.xml +++ b/res/values-gl/strings.xml @@ -388,10 +388,6 @@ Por favor, envía os erros detectados, contribúe con novas funcionalidas e preg Tipo de Seguridade Tipo de autentificación Ningunha - SSL/TLS (se dispoñible) - SSL/TLS (sempre) - STARTTLS (se dispoñible) - STARTTLS (sempre) Borrado de mesaxes Non borrar do servidor diff --git a/res/values-hu/strings.xml b/res/values-hu/strings.xml index 2c915e277..1d9f8b661 100644 --- a/res/values-hu/strings.xml +++ b/res/values-hu/strings.xml @@ -389,10 +389,6 @@ Hibajelentéseivel hozzájárul az újabb verziók tökéletesítéséhez, kérd Kapcsolat biztonsága Hitelesítés típus Nincs - SSL/TLS (ha elérhető) - SSL/TLS (mindig) - STARTTLS (ha elérhető) - STARTTLS (mindig) Üzenet törlésekor Ne törlődjön a szerverről diff --git a/res/values-it/strings.xml b/res/values-it/strings.xml index 4718e0cf0..790138f30 100644 --- a/res/values-it/strings.xml +++ b/res/values-it/strings.xml @@ -388,10 +388,6 @@ Invia le tue segnalazioni, suggerisci nuove funzionalità e chiedi informazioni Tipo di protezione Tipo di autenticazione Nessuna - SSL/TLS (se disponibile) - SSL/TLS (sempre) - STARTTLS (se disponibile) - STARTTLS (sempre) Quando si elimina un messaggio Non eliminare dal server diff --git a/res/values-iw/strings.xml b/res/values-iw/strings.xml index c203bd31d..1d81f1e01 100644 --- a/res/values-iw/strings.xml +++ b/res/values-iw/strings.xml @@ -390,10 +390,6 @@ סוג אבטחה סוג אימות כלום - SSL/TLS (אם זמין) - SSL/TLS (תמיד) - STARTTLS (אם זמין) - STARTTLS (תמיד) כאשר אני מוחק הודעה אל תמחוק בשרת diff --git a/res/values-ja/strings.xml b/res/values-ja/strings.xml index 26001a089..4c0d10ff9 100644 --- a/res/values-ja/strings.xml +++ b/res/values-ja/strings.xml @@ -389,10 +389,6 @@ K-9 は大多数のメールクライアントと同様に、ほとんどのフ 保護された接続 認証タイプ 使用しない - 可能なら SSL/TLS を使用する - SSL/TLS を使用する - 可能なら STARTTLS を使用する - STARTTLS を使用する メール削除時の動作 サーバでは削除しない diff --git a/res/values-ko/strings.xml b/res/values-ko/strings.xml index be8001f93..8ce2a8600 100644 --- a/res/values-ko/strings.xml +++ b/res/values-ko/strings.xml @@ -387,10 +387,6 @@ K-9 메일은 대부분의 무료 hotmail 계정을 지원하지 않으며, 다 보안 연결 인증 방식 없음 - SSL/TLS (유효할 경우) - SSL/TLS (모든 인증서 허용) - STARTTLS (유효할 경우) - STARTTLS (모든 인증서 허용) 메시지 삭제 시 서버에는 메일을 삭제하지 않음 diff --git a/res/values-lt/strings.xml b/res/values-lt/strings.xml index f6650d610..39a5e9281 100644 --- a/res/values-lt/strings.xml +++ b/res/values-lt/strings.xml @@ -388,10 +388,6 @@ Praneškite apie klaidas, pridėkite naujų galimybių ir užduokite klausimus m Saugumas Tapatumo nustatymas Joks - SSL/TLS (jei įmanoma) - SSL/TLS (visada) - STARTTLS (jei įmanoma) - STARTTLS (visada) Kai pašalinu laišką Nešalinti serveryje diff --git a/res/values-nl/strings.xml b/res/values-nl/strings.xml index e20001522..3e2e94c3a 100644 --- a/res/values-nl/strings.xml +++ b/res/values-nl/strings.xml @@ -388,10 +388,6 @@ Graag foutrapporten, bijdrage nieuwe functies en vragen stellen op Beveiligings type Authenticatie type Geen - SSL/TLS (indien beschikbaar) - SSL/TLS (altijd) - STARTTLS (indien beschikbaar) - STARTTLS (altijd) Wanneer ik een bericht verwijder Verwijder niet van server diff --git a/res/values-pl/strings.xml b/res/values-pl/strings.xml index 196d73e94..882d2f5a5 100644 --- a/res/values-pl/strings.xml +++ b/res/values-pl/strings.xml @@ -399,10 +399,6 @@ Wszelkie zgłoszenia usterek, zapytania oraz nowe pomysły prosimy przesyłać z Zabezpieczenia Rodzaj uwierzytelnienia Brak - SSL/TLS (jeśli dostępne) - SSL/TLS (zawsze) - STARTTLS (jeśli dostępne) - STARTTLS (zawsze) Gdy skasuję wiadomość Nie usuwaj z serwera diff --git a/res/values-pt-rBR/strings.xml b/res/values-pt-rBR/strings.xml index 0f268d1c1..cddd5feb4 100644 --- a/res/values-pt-rBR/strings.xml +++ b/res/values-pt-rBR/strings.xml @@ -388,10 +388,6 @@ Por favor, nos envie relatórios de bugs, contribua para novas melhorias e faça Tipo de segurança Tipo de autenticação Nenhum - SSL/TLS (se disponível) - SSL/TLS (sempre) - STARTTLS (se disponível) - STARTTLS (sempre) Quando eu excluir uma mensagem Não excluí-la do servidor diff --git a/res/values-ru/strings.xml b/res/values-ru/strings.xml index 371472f61..714ba95b4 100644 --- a/res/values-ru/strings.xml +++ b/res/values-ru/strings.xml @@ -389,10 +389,6 @@ K-9 Mail — почтовый клиент для Android. Безопасность Аутентификация Нет - SSL/TLS (если доступно) - SSL/TLS (всегда) - STARTTLS (если доступно) - STARTTLS (всегда) Удалённое, на сервере Оставить diff --git a/res/values-sk/strings.xml b/res/values-sk/strings.xml index 6641d617f..c57951a7b 100644 --- a/res/values-sk/strings.xml +++ b/res/values-sk/strings.xml @@ -388,10 +388,6 @@ Prosím, nahlasujte prípadné chyby, prispievajte novými funkciami a pýtajte Zabezpečenie Overenie Žiadne - SSL/TLS (ak je k dispozícii) - SSL/TLS (vždy) - STARTTLS (ak je k dispozícii) - STARTTLS (vždy) Akcia po vymazaní správy Ponechať na serveri diff --git a/res/values-sv/strings.xml b/res/values-sv/strings.xml index 47cc1ba23..8c57b59d3 100644 --- a/res/values-sv/strings.xml +++ b/res/values-sv/strings.xml @@ -389,10 +389,6 @@ Vänligen skicka felrapporter, hjälp till med nya funktioner och ställ frågor Säkerhetstyp Autentiseringstyp Ingen - SSL/TLS (om tillgängligt) - SSL/TLS (alltid) - STARTTLS (om tillgängligt) - STARTTLS (alltid) När jag raderar ett brev Radera inte på servern diff --git a/res/values-tr/strings.xml b/res/values-tr/strings.xml index 152c515af..7f889465f 100644 --- a/res/values-tr/strings.xml +++ b/res/values-tr/strings.xml @@ -388,10 +388,6 @@ Lütfen hata raporlarınızı, istediğiniz yeni özellikleri ve sorularınızı Güvenlik tipi Kimlik doğrulama tipi Hiçbiri - SSL/TLS (Varsa) - SSL/TLS (daima) - STARTTLS (Varsa) - STARTTLS (Daima) Bir mesaj sildiğim zaman Sunucudan silme diff --git a/res/values-uk/strings.xml b/res/values-uk/strings.xml index 4b8886260..2e640af3d 100644 --- a/res/values-uk/strings.xml +++ b/res/values-uk/strings.xml @@ -388,10 +388,6 @@ K-9 Mail це поштовий клієнт з відкритим вихідни Тип системи захисту Метод автентифікації Немає - SSL/TLS (якщо доступно) - SSL/TLS (завжди) - STARTTLS (якщо доступно) - STARTTLS (завжди) Коли повідомлення видалено Не видаляти на сервері diff --git a/res/values-zh-rCN/strings.xml b/res/values-zh-rCN/strings.xml index 0b9b941e5..602db93bc 100644 --- a/res/values-zh-rCN/strings.xml +++ b/res/values-zh-rCN/strings.xml @@ -387,10 +387,6 @@ K-9改进的功能包括： 加密方法 身份验证方法 无 - SSL/TLS（如果可能的话） - SSL/TLS（总是） - STARTTLS（如果可能的话） - STARTTLS（总是） 当我删除邮件时 不要从服务器上删除 diff --git a/res/values-zh-rTW/strings.xml b/res/values-zh-rTW/strings.xml index 3eb6532b3..ce35a86ad 100644 --- a/res/values-zh-rTW/strings.xml +++ b/res/values-zh-rTW/strings.xml @@ -361,10 +361,6 @@ 加密類型 身份驗證類型 無 - SSL/TLS（如果可用） - SSL/TLS（預設） - STARTTLS（如果可用） - STARTTLS（預設） 當我刪除郵件時 不要從伺服器上刪除 diff --git a/res/values/strings.xml b/res/values/strings.xml index 5a1c8103c..21962c369 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -399,10 +399,8 @@ Please submit bug reports, contribute new features and ask questions at Security Authentication None - SSL/TLS (if available) - SSL/TLS (always) - STARTTLS (if available) - STARTTLS (always) + SSL/TLS + STARTTLS When I delete a message Do not delete on server diff --git a/res/xml/providers.xml b/res/xml/providers.xml index 9ecbda571..ef28ec7c4 100644 --- a/res/xml/providers.xml +++ b/res/xml/providers.xml @@ -36,16 +36,12 @@ Valid incoming uri schemes are: imap IMAP with no transport security. - imap+tls IMAP with optional TLS transport security. - If TLS is not available the connection is made as "imap" imap+tls+ IMAP with required TLS transport security. If TLS is not available the conneciton fails. imap+ssl+ IMAP with required SSL transport security. If SSL is not available the connection fails. pop3 POP3 with no transport security. - pop3+tls POP3 with optional TLS transport security. - If TLS is not available the connection is made as "pop3" pop3+tls+ POP3 with required TLS transport security. If TLS is not available the conneciton fails. pop3+ssl+ POP3 with required SSL transport security. @@ -53,8 +49,6 @@ Valid outgoing uri schemes are: smtp SMTP with no transport security. - smtp+tls SMTP with optional TLS transport security. - If TLS is not available the connection is made as "smtp" smtp+tls+ SMTP with required TLS transport security. If TLS is not available the conneciton fails. smtp+ssl+ SMTP with required SSL transport security. @@ -127,8 +121,8 @@ - - + + @@ -154,10 +148,9 @@ - - - + + @@ -296,12 +289,12 @@ - - + + - - + + @@ -323,28 +316,18 @@ - - + + - - + + - - - - - - - - - - + + @@ -538,11 +521,11 @@ - - + + - - + + diff --git a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java index f05376499..9cd7243e3 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java @@ -327,7 +327,6 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener case NONE: port = mDefaultPort; break; - case STARTTLS_OPTIONAL: case STARTTLS_REQUIRED: if (WebDavStore.STORE_TYPE.equals(mStoreType)) { /* @@ -340,7 +339,6 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener port = mDefaultPort; } break; - case SSL_TLS_OPTIONAL: case SSL_TLS_REQUIRED: port = mDefaultSslPort; break; @@ -354,7 +352,6 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener private void updateAuthPlainTextFromSecurityType(ConnectionSecurity securityType) { switch (securityType) { case NONE: - case STARTTLS_OPTIONAL: AuthType.PLAIN.useInsecureText(true, mAuthTypeAdapter); break; default: diff --git a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java index bb77aef11..7aadd43fd 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java @@ -235,11 +235,9 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, String port; switch (securityType) { case NONE: - case STARTTLS_OPTIONAL: case STARTTLS_REQUIRED: port = SMTP_PORT; break; - case SSL_TLS_OPTIONAL: case SSL_TLS_REQUIRED: port = SMTP_SSL_PORT; break; @@ -253,7 +251,6 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, private void updateAuthPlainTextFromSecurityType(ConnectionSecurity securityType) { switch (securityType) { case NONE: - case STARTTLS_OPTIONAL: AuthType.PLAIN.useInsecureText(true, mAuthTypeAdapter); break; default: diff --git a/src/com/fsck/k9/mail/ConnectionSecurity.java b/src/com/fsck/k9/mail/ConnectionSecurity.java index 42ba3ee1a..510eda039 100644 --- a/src/com/fsck/k9/mail/ConnectionSecurity.java +++ b/src/com/fsck/k9/mail/ConnectionSecurity.java @@ -5,9 +5,7 @@ import com.fsck.k9.R; public enum ConnectionSecurity { NONE(R.string.account_setup_incoming_security_none_label), - STARTTLS_OPTIONAL(R.string.account_setup_incoming_security_tls_optional_label), STARTTLS_REQUIRED(R.string.account_setup_incoming_security_tls_label), - SSL_TLS_OPTIONAL(R.string.account_setup_incoming_security_ssl_optional_label), SSL_TLS_REQUIRED(R.string.account_setup_incoming_security_ssl_label); private final int mResourceId; diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index 62b22b604..d52add785 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -146,11 +146,9 @@ public class ImapStore extends Store { * *

Possible forms:

* 
-     * imap://auth:user:password@server:port CONNECTION_SECURITY_NONE
-     * imap+tls://auth:user:password@server:port CONNECTION_SECURITY_TLS_OPTIONAL
-     * imap+tls+://auth:user:password@server:port CONNECTION_SECURITY_TLS_REQUIRED
-     * imap+ssl+://auth:user:password@server:port CONNECTION_SECURITY_SSL_REQUIRED
-     * imap+ssl://auth:user:password@server:port CONNECTION_SECURITY_SSL_OPTIONAL
+     * imap://auth:user:password@server:port ConnectionSecurity.NONE
+     * imap+tls+://auth:user:password@server:port ConnectionSecurity.STARTTLS_REQUIRED
+     * imap+ssl+://auth:user:password@server:port ConnectionSecurity.SSL_TLS_REQUIRED
      *
*/ public static ImapStoreSettings decodeUri(String uri) { @@ -171,21 +169,27 @@ public class ImapStore extends Store { } String scheme = imapUri.getScheme(); + /* + * Currently available schemes are: + * imap + * imap+tls+ + * imap+ssl+ + * + * The following are obsolete schemes that may be found in pre-existing + * settings from earlier versions or that may be found when imported. We + * continue to recognize them and re-map them appropriately: + * imap+tls + * imap+ssl + */ if (scheme.equals("imap")) { connectionSecurity = ConnectionSecurity.NONE; port = 143; - } else if (scheme.equals("imap+tls")) { - connectionSecurity = ConnectionSecurity.STARTTLS_OPTIONAL; - port = 143; - } else if (scheme.equals("imap+tls+")) { + } else if (scheme.startsWith("imap+tls")) { connectionSecurity = ConnectionSecurity.STARTTLS_REQUIRED; port = 143; - } else if (scheme.equals("imap+ssl+")) { + } else if (scheme.startsWith("imap+ssl")) { connectionSecurity = ConnectionSecurity.SSL_TLS_REQUIRED; port = 993; - } else if (scheme.equals("imap+ssl")) { - connectionSecurity = ConnectionSecurity.SSL_TLS_OPTIONAL; - port = 993; } else { throw new IllegalArgumentException("Unsupported protocol (" + scheme + ")"); } @@ -267,15 +271,9 @@ public class ImapStore extends Store { String scheme; switch (server.connectionSecurity) { - case SSL_TLS_OPTIONAL: - scheme = "imap+ssl"; - break; case SSL_TLS_REQUIRED: scheme = "imap+ssl+"; break; - case STARTTLS_OPTIONAL: - scheme = "imap+tls"; - break; case STARTTLS_REQUIRED: scheme = "imap+tls+"; break; @@ -2420,15 +2418,13 @@ public class ImapStore extends Store { SocketAddress socketAddress = new InetSocketAddress(addresses[i], mSettings.getPort()); - if (connectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED || - connectionSecurity == ConnectionSecurity.SSL_TLS_OPTIONAL) { + if (connectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED) { SSLContext sslContext = SSLContext.getInstance("TLS"); - boolean secure = connectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED; sslContext .init(null, new TrustManager[] { TrustManagerFactory.get( mSettings.getHost(), - mSettings.getPort(), secure) }, + mSettings.getPort(), true) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext); } else { @@ -2473,19 +2469,17 @@ public class ImapStore extends Store { } } - if (mSettings.getConnectionSecurity() == ConnectionSecurity.STARTTLS_OPTIONAL - || mSettings.getConnectionSecurity() == ConnectionSecurity.STARTTLS_REQUIRED) { + if (mSettings.getConnectionSecurity() == ConnectionSecurity.STARTTLS_REQUIRED) { if (hasCapability("STARTTLS")) { // STARTTLS executeSimpleCommand("STARTTLS"); SSLContext sslContext = SSLContext.getInstance("TLS"); - boolean secure = mSettings.getConnectionSecurity() == ConnectionSecurity.STARTTLS_REQUIRED; sslContext.init(null, new TrustManager[] { TrustManagerFactory.get( mSettings.getHost(), - mSettings.getPort(), secure) }, + mSettings.getPort(), true) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext, mSocket, mSettings.getHost(), mSettings.getPort(), true); @@ -2502,7 +2496,7 @@ public class ImapStore extends Store { if (responses.size() != 2) { throw new MessagingException("Invalid CAPABILITY response received"); } - } else if (mSettings.getConnectionSecurity() == ConnectionSecurity.STARTTLS_REQUIRED) { + } else { throw new MessagingException("TLS not supported but required"); } } diff --git a/src/com/fsck/k9/mail/store/Pop3Store.java b/src/com/fsck/k9/mail/store/Pop3Store.java index 8acc7e079..e59cbb4ed 100644 --- a/src/com/fsck/k9/mail/store/Pop3Store.java +++ b/src/com/fsck/k9/mail/store/Pop3Store.java @@ -62,11 +62,9 @@ public class Pop3Store extends Store { * *

Possible forms:

* 
-     * pop3://user:password@server:port CONNECTION_SECURITY_NONE
-     * pop3+tls://user:password@server:port CONNECTION_SECURITY_TLS_OPTIONAL
-     * pop3+tls+://user:password@server:port CONNECTION_SECURITY_TLS_REQUIRED
-     * pop3+ssl+://user:password@server:port CONNECTION_SECURITY_SSL_REQUIRED
-     * pop3+ssl://user:password@server:port CONNECTION_SECURITY_SSL_OPTIONAL
+     * pop3://user:password@server:port ConnectionSecurity.NONE
+     * pop3+tls+://user:password@server:port ConnectionSecurity.STARTTLS_REQUIRED
+     * pop3+ssl+://user:password@server:port ConnectionSecurity.SSL_TLS_REQUIRED
      *
*/ public static ServerSettings decodeUri(String uri) { @@ -84,21 +82,27 @@ public class Pop3Store extends Store { } String scheme = pop3Uri.getScheme(); + /* + * Currently available schemes are: + * pop3 + * pop3+tls+ + * pop3+ssl+ + * + * The following are obsolete schemes that may be found in pre-existing + * settings from earlier versions or that may be found when imported. We + * continue to recognize them and re-map them appropriately: + * pop3+tls + * pop3+ssl + */ if (scheme.equals("pop3")) { connectionSecurity = ConnectionSecurity.NONE; port = 110; - } else if (scheme.equals("pop3+tls")) { - connectionSecurity = ConnectionSecurity.STARTTLS_OPTIONAL; - port = 110; - } else if (scheme.equals("pop3+tls+")) { + } else if (scheme.startsWith("pop3+tls")) { connectionSecurity = ConnectionSecurity.STARTTLS_REQUIRED; port = 110; - } else if (scheme.equals("pop3+ssl+")) { + } else if (scheme.startsWith("pop3+ssl")) { connectionSecurity = ConnectionSecurity.SSL_TLS_REQUIRED; port = 995; - } else if (scheme.equals("pop3+ssl")) { - connectionSecurity = ConnectionSecurity.SSL_TLS_OPTIONAL; - port = 995; } else { throw new IllegalArgumentException("Unsupported protocol (" + scheme + ")"); } @@ -161,15 +165,9 @@ public class Pop3Store extends Store { String scheme; switch (server.connectionSecurity) { - case SSL_TLS_OPTIONAL: - scheme = "pop3+ssl"; - break; case SSL_TLS_REQUIRED: scheme = "pop3+ssl+"; break; - case STARTTLS_OPTIONAL: - scheme = "pop3+tls"; - break; case STARTTLS_REQUIRED: scheme = "pop3+tls+"; break; @@ -299,13 +297,11 @@ public class Pop3Store extends Store { try { SocketAddress socketAddress = new InetSocketAddress(mHost, mPort); - if (mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED || - mConnectionSecurity == ConnectionSecurity.SSL_TLS_OPTIONAL) { + if (mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED) { SSLContext sslContext = SSLContext.getInstance("TLS"); - final boolean secure = mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED; sslContext.init(null, new TrustManager[] { TrustManagerFactory.get(mHost, - mPort, secure) }, new SecureRandom()); + mPort, true) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext); } else { mSocket = new Socket(); @@ -323,17 +319,15 @@ public class Pop3Store extends Store { String serverGreeting = executeSimpleCommand(null); mCapabilities = getCapabilities(); - if (mConnectionSecurity == ConnectionSecurity.STARTTLS_OPTIONAL - || mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { + if (mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { if (mCapabilities.stls) { executeSimpleCommand(STLS_COMMAND); SSLContext sslContext = SSLContext.getInstance("TLS"); - boolean secure = mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED; sslContext.init(null, new TrustManager[] { TrustManagerFactory.get( - mHost, mPort, secure) }, + mHost, mPort, true) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext, mSocket, mHost, mPort, true); @@ -344,7 +338,7 @@ public class Pop3Store extends Store { throw new MessagingException("Unable to connect socket"); } mCapabilities = getCapabilities(); - } else if (mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { + } else { throw new MessagingException("TLS not supported but required"); } } diff --git a/src/com/fsck/k9/mail/store/WebDavStore.java b/src/com/fsck/k9/mail/store/WebDavStore.java index b0af15ef2..2e11e6c8f 100644 --- a/src/com/fsck/k9/mail/store/WebDavStore.java +++ b/src/com/fsck/k9/mail/store/WebDavStore.java @@ -82,11 +82,9 @@ public class WebDavStore extends Store { * *

Possible forms:

* 
-     * webdav://user:password@server:port CONNECTION_SECURITY_NONE
-     * webdav+tls://user:password@server:port CONNECTION_SECURITY_TLS_OPTIONAL
-     * webdav+tls+://user:password@server:port CONNECTION_SECURITY_TLS_REQUIRED
-     * webdav+ssl+://user:password@server:port CONNECTION_SECURITY_SSL_REQUIRED
-     * webdav+ssl://user:password@server:port CONNECTION_SECURITY_SSL_OPTIONAL
+     * webdav://user:password@server:port ConnectionSecurity.NONE
+     * webdav+tls+://user:password@server:port ConnectionSecurity.STARTTLS_REQUIRED
+     * webdav+ssl+://user:password@server:port ConnectionSecurity.SSL_TLS_REQUIRED
      *
*/ public static WebDavStoreSettings decodeUri(String uri) { @@ -109,15 +107,23 @@ public class WebDavStore extends Store { } String scheme = webDavUri.getScheme(); + /* + * Currently available schemes are: + * webdav + * webdav+tls+ + * webdav+ssl+ + * + * The following are obsolete schemes that may be found in pre-existing + * settings from earlier versions or that may be found when imported. We + * continue to recognize them and re-map them appropriately: + * webdav+tls + * webdav+ssl + */ if (scheme.equals("webdav")) { connectionSecurity = ConnectionSecurity.NONE; - } else if (scheme.equals("webdav+ssl")) { - connectionSecurity = ConnectionSecurity.SSL_TLS_OPTIONAL; - } else if (scheme.equals("webdav+ssl+")) { + } else if (scheme.startsWith("webdav+ssl")) { connectionSecurity = ConnectionSecurity.SSL_TLS_REQUIRED; - } else if (scheme.equals("webdav+tls")) { - connectionSecurity = ConnectionSecurity.STARTTLS_OPTIONAL; - } else if (scheme.equals("webdav+tls+")) { + } else if (scheme.startsWith("webdav+tls")) { connectionSecurity = ConnectionSecurity.STARTTLS_REQUIRED; } else { throw new IllegalArgumentException("Unsupported protocol (" + scheme + ")"); @@ -203,15 +209,9 @@ public class WebDavStore extends Store { String scheme; switch (server.connectionSecurity) { - case SSL_TLS_OPTIONAL: - scheme = "webdav+ssl"; - break; case SSL_TLS_REQUIRED: scheme = "webdav+ssl+"; break; - case STARTTLS_OPTIONAL: - scheme = "webdav+tls"; - break; case STARTTLS_REQUIRED: scheme = "webdav+tls+"; break; @@ -367,9 +367,7 @@ public class WebDavStore extends Store { private String getRoot() { String root; if (mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED || - mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED || - mConnectionSecurity == ConnectionSecurity.STARTTLS_OPTIONAL || - mConnectionSecurity == ConnectionSecurity.SSL_TLS_OPTIONAL) { + mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED) { root = "https"; } else { root = "http"; diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index 9c8142b4c..6f96fe430 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -38,11 +38,9 @@ public class SmtpTransport extends Transport { * *

Possible forms:

* 
-     * smtp://user:password@server:port CONNECTION_SECURITY_NONE
-     * smtp+tls://user:password@server:port CONNECTION_SECURITY_TLS_OPTIONAL
-     * smtp+tls+://user:password@server:port CONNECTION_SECURITY_TLS_REQUIRED
-     * smtp+ssl+://user:password@server:port CONNECTION_SECURITY_SSL_REQUIRED
-     * smtp+ssl://user:password@server:port CONNECTION_SECURITY_SSL_OPTIONAL
+     * smtp://user:password@server:port ConnectionSecurity.NONE
+     * smtp+tls+://user:password@server:port ConnectionSecurity.STARTTLS_REQUIRED
+     * smtp+ssl+://user:password@server:port ConnectionSecurity.SSL_TLS_REQUIRED
      *
*/ public static ServerSettings decodeUri(String uri) { @@ -61,21 +59,27 @@ public class SmtpTransport extends Transport { } String scheme = smtpUri.getScheme(); + /* + * Currently available schemes are: + * smtp + * smtp+tls+ + * smtp+ssl+ + * + * The following are obsolete schemes that may be found in pre-existing + * settings from earlier versions or that may be found when imported. We + * continue to recognize them and re-map them appropriately: + * smtp+tls + * smtp+ssl + */ if (scheme.equals("smtp")) { connectionSecurity = ConnectionSecurity.NONE; port = 587; - } else if (scheme.equals("smtp+tls")) { - connectionSecurity = ConnectionSecurity.STARTTLS_OPTIONAL; - port = 587; - } else if (scheme.equals("smtp+tls+")) { + } else if (scheme.startsWith("smtp+tls")) { connectionSecurity = ConnectionSecurity.STARTTLS_REQUIRED; port = 587; - } else if (scheme.equals("smtp+ssl+")) { + } else if (scheme.startsWith("smtp+ssl")) { connectionSecurity = ConnectionSecurity.SSL_TLS_REQUIRED; port = 465; - } else if (scheme.equals("smtp+ssl")) { - connectionSecurity = ConnectionSecurity.SSL_TLS_OPTIONAL; - port = 465; } else { throw new IllegalArgumentException("Unsupported protocol (" + scheme + ")"); } @@ -132,15 +136,9 @@ public class SmtpTransport extends Transport { String scheme; switch (server.connectionSecurity) { - case SSL_TLS_OPTIONAL: - scheme = "smtp+ssl"; - break; case SSL_TLS_REQUIRED: scheme = "smtp+ssl+"; break; - case STARTTLS_OPTIONAL: - scheme = "smtp+tls"; - break; case STARTTLS_REQUIRED: scheme = "smtp+tls+"; break; @@ -202,13 +200,11 @@ public class SmtpTransport extends Transport { for (int i = 0; i < addresses.length; i++) { try { SocketAddress socketAddress = new InetSocketAddress(addresses[i], mPort); - if (mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED || - mConnectionSecurity == ConnectionSecurity.SSL_TLS_OPTIONAL) { + if (mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED) { SSLContext sslContext = SSLContext.getInstance("TLS"); - boolean secure = mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED; sslContext.init(null, new TrustManager[] { TrustManagerFactory.get( - mHost, mPort, secure) }, + mHost, mPort, true) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext); mSocket.connect(socketAddress, SOCKET_CONNECT_TIMEOUT); @@ -260,16 +256,14 @@ public class SmtpTransport extends Transport { m8bitEncodingAllowed = extensions.containsKey("8BITMIME"); - if (mConnectionSecurity == ConnectionSecurity.STARTTLS_OPTIONAL - || mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { + if (mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { if (extensions.containsKey("STARTTLS")) { executeSimpleCommand("STARTTLS"); SSLContext sslContext = SSLContext.getInstance("TLS"); - boolean secure = mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED; sslContext.init(null, new TrustManager[] { TrustManagerFactory.get(mHost, - mPort, secure) }, new SecureRandom()); + mPort, true) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext, mSocket, mHost, mPort, true); mIn = new PeekableInputStream(new BufferedInputStream(mSocket.getInputStream(), @@ -281,7 +275,7 @@ public class SmtpTransport extends Transport { */ extensions = sendHello(localHost); secureConnection = true; - } else if (mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { + } else { throw new MessagingException("TLS not supported but required"); } } From 14a0a7a2a7cda7cf385ba1f726c039c445dc9e71 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Fri, 28 Feb 2014 12:47:43 -0500 Subject: [PATCH 17/25] Provide notification if STARTTLS is not available --- src/com/fsck/k9/mail/store/ImapStore.java | 12 +++++++++++- src/com/fsck/k9/mail/store/Pop3Store.java | 13 ++++++++++++- src/com/fsck/k9/mail/transport/SmtpTransport.java | 13 ++++++++++++- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index d52add785..d52e78c71 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -27,6 +27,7 @@ import java.nio.charset.CodingErrorAction; import java.security.GeneralSecurityException; import java.security.SecureRandom; import java.security.Security; +import java.security.cert.CertificateException; import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Arrays; @@ -2497,7 +2498,16 @@ public class ImapStore extends Store { throw new MessagingException("Invalid CAPABILITY response received"); } } else { - throw new MessagingException("TLS not supported but required"); + /* + * This exception triggers a "Certificate error" + * notification that takes the user to the incoming + * server settings for review. This might be needed if + * the account was configured with an obsolete + * "STARTTLS (if available)" setting. + */ + throw new CertificateValidationException( + "STARTTLS connection security not available", + new CertificateException()); } } diff --git a/src/com/fsck/k9/mail/store/Pop3Store.java b/src/com/fsck/k9/mail/store/Pop3Store.java index e59cbb4ed..c5a23183b 100644 --- a/src/com/fsck/k9/mail/store/Pop3Store.java +++ b/src/com/fsck/k9/mail/store/Pop3Store.java @@ -18,12 +18,14 @@ import com.fsck.k9.net.ssl.TrustedSocketFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLException; import javax.net.ssl.TrustManager; + import java.io.*; import java.net.*; import java.security.GeneralSecurityException; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; +import java.security.cert.CertificateException; import java.util.ArrayList; import java.util.Arrays; import java.util.Date; @@ -339,7 +341,16 @@ public class Pop3Store extends Store { } mCapabilities = getCapabilities(); } else { - throw new MessagingException("TLS not supported but required"); + /* + * This exception triggers a "Certificate error" + * notification that takes the user to the incoming + * server settings for review. This might be needed if + * the account was configured with an obsolete + * "STARTTLS (if available)" setting. + */ + throw new CertificateValidationException( + "STARTTLS connection security not available", + new CertificateException()); } } diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index 6f96fe430..dc8a5e8c9 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -19,6 +19,7 @@ import com.fsck.k9.net.ssl.TrustedSocketFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLException; import javax.net.ssl.TrustManager; + import java.io.BufferedInputStream; import java.io.BufferedOutputStream; import java.io.IOException; @@ -27,6 +28,7 @@ import java.io.UnsupportedEncodingException; import java.net.*; import java.security.GeneralSecurityException; import java.security.SecureRandom; +import java.security.cert.CertificateException; import java.util.*; @@ -276,7 +278,16 @@ public class SmtpTransport extends Transport { extensions = sendHello(localHost); secureConnection = true; } else { - throw new MessagingException("TLS not supported but required"); + /* + * This exception triggers a "Certificate error" + * notification that takes the user to the incoming + * server settings for review. This might be needed if + * the account was configured with an obsolete + * "STARTTLS (if available)" setting. + */ + throw new CertificateValidationException( + "STARTTLS connection security not available", + new CertificateException()); } } From 9dc5338501c63526f954e93e55983811e822a6a3 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Wed, 26 Feb 2014 17:59:29 -0500 Subject: [PATCH 18/25] Eliminate WebDAV STARTTLS security choice STARTTLS doesn't really apply to WebDAV and should never have been made available as an option. Pre-existing settings will be re-mapped to SSL/TLS. --- .../activity/setup/AccountSetupIncoming.java | 33 ++++++++----------- src/com/fsck/k9/mail/store/WebDavStore.java | 13 ++------ 2 files changed, 16 insertions(+), 30 deletions(-) diff --git a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java index 9cd7243e3..abecd290e 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java @@ -68,6 +68,7 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener private ArrayAdapter mAuthTypeAdapter; private String mDefaultPort = ""; private String mDefaultSslPort = ""; + private ConnectionSecurity[] mConnectionSecurityChoices = ConnectionSecurity.values(); public static void actionIncomingSettings(Activity context, Account account, boolean makeDefault) { Intent i = new Intent(context, AccountSetupIncoming.class); @@ -124,11 +125,6 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener } }); - ArrayAdapter securityTypesAdapter = new ArrayAdapter(this, - android.R.layout.simple_spinner_item, ConnectionSecurity.values()); - securityTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); - mSecurityTypeView.setAdapter(securityTypesAdapter); - mAuthTypeAdapter = AuthType.getArrayAdapter(this); mAuthTypeView.setAdapter(mAuthTypeAdapter); @@ -189,9 +185,6 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener int position = mAuthTypeAdapter.getPosition(settings.authenticationType); mAuthTypeView.setSelection(position, false); - // Select currently configured security type - mSecurityTypeView.setSelection(settings.connectionSecurity.ordinal(), false); - mStoreType = settings.type; if (Pop3Store.STORE_TYPE.equals(settings.type)) { serverLabelView.setText(R.string.account_setup_incoming_pop_server_label); @@ -231,6 +224,9 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener serverLabelView.setText(R.string.account_setup_incoming_webdav_server_label); mDefaultPort = WEBDAV_PORT; mDefaultSslPort = WEBDAV_SSL_PORT; + mConnectionSecurityChoices = new ConnectionSecurity[] { + ConnectionSecurity.NONE, + ConnectionSecurity.SSL_TLS_REQUIRED }; // Hide the unnecessary fields findViewById(R.id.imap_path_prefix_section).setVisibility(View.GONE); @@ -258,6 +254,14 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener throw new Exception("Unknown account type: " + mAccount.getStoreUri()); } + ArrayAdapter securityTypesAdapter = new ArrayAdapter(this, + android.R.layout.simple_spinner_item, mConnectionSecurityChoices); + securityTypesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item); + mSecurityTypeView.setAdapter(securityTypesAdapter); + + // Select currently configured security type + mSecurityTypeView.setSelection(settings.connectionSecurity.ordinal(), false); + /* * Updates the port when the user changes the security type. This allows * us to show a reasonable default which the user can change. @@ -325,19 +329,8 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener String port; switch (securityType) { case NONE: - port = mDefaultPort; - break; case STARTTLS_REQUIRED: - if (WebDavStore.STORE_TYPE.equals(mStoreType)) { - /* - * The concept of STARTTLS is not really applicable for WebDav and should never - * have been made a user-selectable option. But now we must support the setting - * if it exists. - */ - port = mDefaultSslPort; - } else { - port = mDefaultPort; - } + port = mDefaultPort; break; case SSL_TLS_REQUIRED: port = mDefaultSslPort; diff --git a/src/com/fsck/k9/mail/store/WebDavStore.java b/src/com/fsck/k9/mail/store/WebDavStore.java index 2e11e6c8f..8f75957b4 100644 --- a/src/com/fsck/k9/mail/store/WebDavStore.java +++ b/src/com/fsck/k9/mail/store/WebDavStore.java @@ -83,7 +83,6 @@ public class WebDavStore extends Store { *

Possible forms:

* 
      * webdav://user:password@server:port ConnectionSecurity.NONE
-     * webdav+tls+://user:password@server:port ConnectionSecurity.STARTTLS_REQUIRED
      * webdav+ssl+://user:password@server:port ConnectionSecurity.SSL_TLS_REQUIRED
      *
*/ @@ -110,21 +109,19 @@ public class WebDavStore extends Store { /* * Currently available schemes are: * webdav - * webdav+tls+ * webdav+ssl+ * * The following are obsolete schemes that may be found in pre-existing * settings from earlier versions or that may be found when imported. We * continue to recognize them and re-map them appropriately: * webdav+tls + * webdav+tls+ * webdav+ssl */ if (scheme.equals("webdav")) { connectionSecurity = ConnectionSecurity.NONE; - } else if (scheme.startsWith("webdav+ssl")) { + } else if (scheme.startsWith("webdav+")) { connectionSecurity = ConnectionSecurity.SSL_TLS_REQUIRED; - } else if (scheme.startsWith("webdav+tls")) { - connectionSecurity = ConnectionSecurity.STARTTLS_REQUIRED; } else { throw new IllegalArgumentException("Unsupported protocol (" + scheme + ")"); } @@ -212,9 +209,6 @@ public class WebDavStore extends Store { case SSL_TLS_REQUIRED: scheme = "webdav+ssl+"; break; - case STARTTLS_REQUIRED: - scheme = "webdav+tls+"; - break; default: case NONE: scheme = "webdav"; @@ -366,8 +360,7 @@ public class WebDavStore extends Store { private String getRoot() { String root; - if (mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED || - mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED) { + if (mConnectionSecurity == ConnectionSecurity.SSL_TLS_REQUIRED) { root = "https"; } else { root = "http"; From d67c054d4dc565c7e102ac742c5e14c76a1c66f0 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Fri, 28 Feb 2014 19:33:25 -0500 Subject: [PATCH 19/25] Restart pushers after editing server settings If an error occurs while connecting for push (for example, if the server's certificate fails authentication), several more attempts will be made to connect, then finally all attempts will cease. This commit makes it so that pushers are restarted if the user goes in and edits the server settings (presumably because the user was notified of a problem and is attempting to fix it). Without this, the user could fix the problem and would still not receive email via push. --- .../fsck/k9/activity/setup/AccountSetupIncoming.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java index abecd290e..b275e005d 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java @@ -15,6 +15,7 @@ import android.widget.*; import android.widget.CompoundButton.OnCheckedChangeListener; import com.fsck.k9.*; +import com.fsck.k9.Account.FolderMode; import com.fsck.k9.activity.K9Activity; import com.fsck.k9.activity.setup.AccountSetupCheckSettings.CheckDirection; import com.fsck.k9.helper.Utility; @@ -27,6 +28,7 @@ import com.fsck.k9.mail.store.Pop3Store; import com.fsck.k9.mail.store.WebDavStore; import com.fsck.k9.mail.store.ImapStore.ImapStoreSettings; import com.fsck.k9.mail.store.WebDavStore.WebDavStoreSettings; +import com.fsck.k9.service.MailService; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -356,6 +358,16 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener public void onActivityResult(int requestCode, int resultCode, Intent data) { if (resultCode == RESULT_OK) { if (Intent.ACTION_EDIT.equals(getIntent().getAction())) { + boolean isPushCapable = false; + try { + Store store = mAccount.getRemoteStore(); + isPushCapable = store.isPushCapable(); + } catch (Exception e) { + Log.e(K9.LOG_TAG, "Could not get remote store", e); + } + if (isPushCapable && mAccount.getFolderPushMode() != FolderMode.NONE) { + MailService.actionRestartPushers(this, null); + } mAccount.save(Preferences.getPreferences(this)); finish(); } else { From c8150a12fa839475d66c496d160f479a21b98501 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Sat, 1 Mar 2014 11:27:24 -0500 Subject: [PATCH 20/25] Show account name in certificate error notifications getName() shows the user's name from the first identity for the account. What we really want is getDescription(), which is the account name that shows in the account list. --- src/com/fsck/k9/controller/MessagingController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/com/fsck/k9/controller/MessagingController.java b/src/com/fsck/k9/controller/MessagingController.java index d1fdf20c0..f85bee3f2 100644 --- a/src/com/fsck/k9/controller/MessagingController.java +++ b/src/com/fsck/k9/controller/MessagingController.java @@ -2647,7 +2647,7 @@ public class MessagingController implements Runnable { final PendingIntent pi = PendingIntent.getActivity(context, account.getAccountNumber(), i, PendingIntent.FLAG_UPDATE_CURRENT); final String title = context.getString( - R.string.notification_certificate_error_title, account.getName()); + R.string.notification_certificate_error_title, account.getDescription()); final NotificationCompat.Builder builder = new NotificationBuilder(context); builder.setSmallIcon(R.drawable.ic_notify_new_mail); From df3eef0052d4d11d626a568af572aed356854d4d Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Thu, 27 Feb 2014 13:06:54 -0500 Subject: [PATCH 21/25] Fix the K9mail-errors folder Error messages were not being put in the folder because of a problem with how loopCatch was being handled. It looks like this problem goes back to commit 5aea9e7. Additional device info has been added to the error messages. Also, now the feature is only enabled in debug mode. --- src/com/fsck/k9/K9.java | 3 ++- .../k9/controller/MessagingController.java | 19 +++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/com/fsck/k9/K9.java b/src/com/fsck/k9/K9.java index 74d3fd29b..9a841a5c7 100644 --- a/src/com/fsck/k9/K9.java +++ b/src/com/fsck/k9/K9.java @@ -177,8 +177,9 @@ public class K9 extends Application { /** * Can create messages containing stack traces that can be forwarded * to the development team. + * + * Feature is enabled when DEBUG == true */ - public static boolean ENABLE_ERROR_FOLDER = true; public static String ERROR_FOLDER_NAME = "K9mail-errors"; /** diff --git a/src/com/fsck/k9/controller/MessagingController.java b/src/com/fsck/k9/controller/MessagingController.java index f85bee3f2..2fa6d3fd5 100644 --- a/src/com/fsck/k9/controller/MessagingController.java +++ b/src/com/fsck/k9/controller/MessagingController.java @@ -27,6 +27,7 @@ import android.app.PendingIntent; import android.content.ContentResolver; import android.content.Context; import android.content.Intent; +import android.content.pm.PackageInfo; import android.database.Cursor; import android.net.Uri; import android.os.Build; @@ -2683,9 +2684,6 @@ public class MessagingController implements Runnable { static long uidfill = 0; static AtomicBoolean loopCatch = new AtomicBoolean(); public void addErrorMessage(Account account, String subject, Throwable t) { - if (!loopCatch.compareAndSet(false, true)) { - return; - } try { if (t == null) { return; @@ -2693,6 +2691,17 @@ public class MessagingController implements Runnable { CharArrayWriter baos = new CharArrayWriter(t.getStackTrace().length * 10); PrintWriter ps = new PrintWriter(baos); + try { + Application context = K9.app; + PackageInfo packageInfo = context.getPackageManager().getPackageInfo( + context.getPackageName(), 0); + ps.format("K9-Mail version: %s\r\n", packageInfo.versionName); + } catch (Exception e) { + // ignore + } + ps.format("Device make: %s\r\n", Build.MANUFACTURER); + ps.format("Device model: %s\r\n", Build.MODEL); + ps.format("Android version: %s\r\n\r\n", Build.VERSION.RELEASE); t.printStackTrace(ps); ps.close(); @@ -2703,13 +2712,11 @@ public class MessagingController implements Runnable { addErrorMessage(account, subject, baos.toString()); } catch (Throwable it) { Log.e(K9.LOG_TAG, "Could not save error message to " + account.getErrorFolderName(), it); - } finally { - loopCatch.set(false); } } public void addErrorMessage(Account account, String subject, String body) { - if (!K9.ENABLE_ERROR_FOLDER) { + if (!K9.DEBUG) { return; } if (!loopCatch.compareAndSet(false, true)) { From 094318dacbb478de4c4a403b4ebce2bfe65f75f9 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Sat, 1 Mar 2014 17:19:36 -0500 Subject: [PATCH 22/25] Use buffered output streams for all output POP3 already does this. This is a more general solution to the problem addressed in commit 8bfd6ca. --- src/com/fsck/k9/mail/store/ImapStore.java | 6 ++---- src/com/fsck/k9/mail/transport/SmtpTransport.java | 7 +++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index d52e78c71..3a28951c7 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -2450,7 +2450,7 @@ public class ImapStore extends Store { mIn = new PeekableInputStream(new BufferedInputStream(mSocket.getInputStream(), 1024)); mParser = new ImapResponseParser(mIn); - mOut = mSocket.getOutputStream(); + mOut = new BufferedOutputStream(mSocket.getOutputStream(), 1024); capabilities.clear(); ImapResponse nullResponse = mParser.readResponse(); @@ -2488,7 +2488,7 @@ public class ImapStore extends Store { mIn = new PeekableInputStream(new BufferedInputStream(mSocket .getInputStream(), 1024)); mParser = new ImapResponseParser(mIn); - mOut = mSocket.getOutputStream(); + mOut = new BufferedOutputStream(mSocket.getOutputStream(), 1024); // Per RFC 2595 (3.1): Once TLS has been started, reissue CAPABILITY command if (K9.DEBUG) Log.i(K9.LOG_TAG, "Updating capabilities after STARTTLS for " + getLogId()); @@ -2511,8 +2511,6 @@ public class ImapStore extends Store { } } - mOut = new BufferedOutputStream(mOut, 1024); - switch (mSettings.getAuthType()) { case CRAM_MD5: if (hasCapability(CAPABILITY_AUTH_CRAM_MD5)) { diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index dc8a5e8c9..adce9b181 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -229,7 +229,7 @@ public class SmtpTransport extends Transport { mSocket.setSoTimeout(SOCKET_READ_TIMEOUT); mIn = new PeekableInputStream(new BufferedInputStream(mSocket.getInputStream(), 1024)); - mOut = mSocket.getOutputStream(); + mOut = new BufferedOutputStream(mSocket.getOutputStream(), 1024); // Eat the banner executeSimpleCommand(null); @@ -270,7 +270,7 @@ public class SmtpTransport extends Transport { mPort, true); mIn = new PeekableInputStream(new BufferedInputStream(mSocket.getInputStream(), 1024)); - mOut = mSocket.getOutputStream(); + mOut = new BufferedOutputStream(mSocket.getOutputStream(), 1024); /* * Now resend the EHLO. Required by RFC2487 Sec. 5.2, and more specifically, * Exim. @@ -493,8 +493,7 @@ public class SmtpTransport extends Transport { executeSimpleCommand("DATA"); EOLConvertingOutputStream msgOut = new EOLConvertingOutputStream( - new LineWrapOutputStream(new SmtpDataStuffing( - new BufferedOutputStream(mOut, 1024)), 1000)); + new LineWrapOutputStream(new SmtpDataStuffing(mOut), 1000)); message.writeTo(msgOut); From 0a63466704ce0ccbc5f6ad65d3940e6b5b2aba2a Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 5 Mar 2014 04:16:57 +0100 Subject: [PATCH 23/25] Add missing import --- src/com/fsck/k9/mail/store/Pop3Store.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/com/fsck/k9/mail/store/Pop3Store.java b/src/com/fsck/k9/mail/store/Pop3Store.java index c5a23183b..0c44b21a5 100644 --- a/src/com/fsck/k9/mail/store/Pop3Store.java +++ b/src/com/fsck/k9/mail/store/Pop3Store.java @@ -33,6 +33,7 @@ import java.util.LinkedList; import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; From 75fc76773d0512a0fb62b213d154bdc3e21bb260 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 5 Mar 2014 05:32:00 +0100 Subject: [PATCH 24/25] Make sure settings import doesn't degrade connection security Rewrite the now obsolete values "SSL_TLS_OPTIONAL" and "STARTTLS_OPTIONAL" to their "*_REQUIRED" counterparts before import. --- src/com/fsck/k9/preferences/SettingsImporter.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/com/fsck/k9/preferences/SettingsImporter.java b/src/com/fsck/k9/preferences/SettingsImporter.java index 9a77614b0..a03a48cdd 100644 --- a/src/com/fsck/k9/preferences/SettingsImporter.java +++ b/src/com/fsck/k9/preferences/SettingsImporter.java @@ -1110,6 +1110,16 @@ public class SettingsImporter { private static ConnectionSecurity convertConnectionSecurity(String connectionSecurity) { try { + /* + * TODO: + * Add proper settings validation and upgrade capability for server settings. + * Once that exists, move this code into a SettingsUpgrader. + */ + if ("SSL_TLS_OPTIONAL".equals(connectionSecurity)) { + return ConnectionSecurity.SSL_TLS_REQUIRED; + } else if ("STARTTLS_OPTIONAL".equals(connectionSecurity)) { + return ConnectionSecurity.STARTTLS_REQUIRED; + } return ConnectionSecurity.valueOf(connectionSecurity); } catch (Exception e) { return ConnectionSecurity.NONE; From 617123c58b7f8a126206b212123f417b1a087974 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 5 Mar 2014 06:03:06 +0100 Subject: [PATCH 25/25] Remove SimpleX509TrustManager because it's no longer used --- src/com/fsck/k9/mail/store/ImapStore.java | 4 ++-- src/com/fsck/k9/mail/store/Pop3Store.java | 4 ++-- .../k9/mail/store/WebDavSocketFactory.java | 2 +- .../fsck/k9/mail/transport/SmtpTransport.java | 4 ++-- .../fsck/k9/net/ssl/TrustManagerFactory.java | 20 ++-------------- .../k9/net/ssl/TrustManagerFactoryTest.java | 24 +++++++++---------- 6 files changed, 21 insertions(+), 37 deletions(-) diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index 3a28951c7..74fdd453f 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -2425,7 +2425,7 @@ public class ImapStore extends Store { .init(null, new TrustManager[] { TrustManagerFactory.get( mSettings.getHost(), - mSettings.getPort(), true) }, + mSettings.getPort()) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext); } else { @@ -2480,7 +2480,7 @@ public class ImapStore extends Store { sslContext.init(null, new TrustManager[] { TrustManagerFactory.get( mSettings.getHost(), - mSettings.getPort(), true) }, + mSettings.getPort()) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext, mSocket, mSettings.getHost(), mSettings.getPort(), true); diff --git a/src/com/fsck/k9/mail/store/Pop3Store.java b/src/com/fsck/k9/mail/store/Pop3Store.java index 0c44b21a5..a886ca0c0 100644 --- a/src/com/fsck/k9/mail/store/Pop3Store.java +++ b/src/com/fsck/k9/mail/store/Pop3Store.java @@ -304,7 +304,7 @@ public class Pop3Store extends Store { SSLContext sslContext = SSLContext.getInstance("TLS"); sslContext.init(null, new TrustManager[] { TrustManagerFactory.get(mHost, - mPort, true) }, new SecureRandom()); + mPort) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext); } else { mSocket = new Socket(); @@ -330,7 +330,7 @@ public class Pop3Store extends Store { SSLContext sslContext = SSLContext.getInstance("TLS"); sslContext.init(null, new TrustManager[] { TrustManagerFactory.get( - mHost, mPort, true) }, + mHost, mPort) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext, mSocket, mHost, mPort, true); diff --git a/src/com/fsck/k9/mail/store/WebDavSocketFactory.java b/src/com/fsck/k9/mail/store/WebDavSocketFactory.java index 9563f510e..73f110e4e 100644 --- a/src/com/fsck/k9/mail/store/WebDavSocketFactory.java +++ b/src/com/fsck/k9/mail/store/WebDavSocketFactory.java @@ -31,7 +31,7 @@ public class WebDavSocketFactory implements LayeredSocketFactory { public WebDavSocketFactory(String host, int port, boolean secure) throws NoSuchAlgorithmException, KeyManagementException { SSLContext sslContext = SSLContext.getInstance("TLS"); sslContext.init(null, new TrustManager[] { - TrustManagerFactory.get(host, port, secure) + TrustManagerFactory.get(host, port) }, new SecureRandom()); mSocketFactory = sslContext.getSocketFactory(); mSchemeSocketFactory = org.apache.http.conn.ssl.SSLSocketFactory.getSocketFactory(); diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index adce9b181..286253b5c 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -206,7 +206,7 @@ public class SmtpTransport extends Transport { SSLContext sslContext = SSLContext.getInstance("TLS"); sslContext.init(null, new TrustManager[] { TrustManagerFactory.get( - mHost, mPort, true) }, + mHost, mPort) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext); mSocket.connect(socketAddress, SOCKET_CONNECT_TIMEOUT); @@ -265,7 +265,7 @@ public class SmtpTransport extends Transport { SSLContext sslContext = SSLContext.getInstance("TLS"); sslContext.init(null, new TrustManager[] { TrustManagerFactory.get(mHost, - mPort, true) }, new SecureRandom()); + mPort) }, new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext, mSocket, mHost, mPort, true); mIn = new PeekableInputStream(new BufferedInputStream(mSocket.getInputStream(), diff --git a/src/com/fsck/k9/net/ssl/TrustManagerFactory.java b/src/com/fsck/k9/net/ssl/TrustManagerFactory.java index 6b6b54138..27b2c70bb 100644 --- a/src/com/fsck/k9/net/ssl/TrustManagerFactory.java +++ b/src/com/fsck/k9/net/ssl/TrustManagerFactory.java @@ -21,23 +21,9 @@ public final class TrustManagerFactory { private static final String LOG_TAG = "TrustManagerFactory"; private static X509TrustManager defaultTrustManager; - private static X509TrustManager unsecureTrustManager; private static LocalKeyStore keyStore; - private static class SimpleX509TrustManager implements X509TrustManager { - public void checkClientTrusted(X509Certificate[] chain, String authType) - throws CertificateException { - } - - public void checkServerTrusted(X509Certificate[] chain, String authType) - throws CertificateException { - } - - public X509Certificate[] getAcceptedIssuers() { - return null; - } - } private static class SecureX509TrustManager implements X509TrustManager { private static final Map mTrustManager = @@ -126,14 +112,12 @@ public final class TrustManagerFactory { } catch (KeyStoreException e) { Log.e(LOG_TAG, "Key Store exception while initializing TrustManagerFactory ", e); } - unsecureTrustManager = new SimpleX509TrustManager(); } private TrustManagerFactory() { } - public static X509TrustManager get(String host, int port, boolean secure) { - return secure ? SecureX509TrustManager.getInstance(host, port) : - unsecureTrustManager; + public static X509TrustManager get(String host, int port) { + return SecureX509TrustManager.getInstance(host, port); } } diff --git a/tests/src/com/fsck/k9/net/ssl/TrustManagerFactoryTest.java b/tests/src/com/fsck/k9/net/ssl/TrustManagerFactoryTest.java index 710009eee..ca33ba106 100644 --- a/tests/src/com/fsck/k9/net/ssl/TrustManagerFactoryTest.java +++ b/tests/src/com/fsck/k9/net/ssl/TrustManagerFactoryTest.java @@ -214,27 +214,27 @@ public class TrustManagerFactoryTest extends AndroidTestCase { mKeyStore.addCertificate(NOT_MATCHING_HOST, PORT1, mCert1); mKeyStore.addCertificate(NOT_MATCHING_HOST, PORT2, mCert2); - X509TrustManager trustManager1 = TrustManagerFactory.get(NOT_MATCHING_HOST, PORT1, true); - X509TrustManager trustManager2 = TrustManagerFactory.get(NOT_MATCHING_HOST, PORT2, true); + X509TrustManager trustManager1 = TrustManagerFactory.get(NOT_MATCHING_HOST, PORT1); + X509TrustManager trustManager2 = TrustManagerFactory.get(NOT_MATCHING_HOST, PORT2); trustManager2.checkServerTrusted(new X509Certificate[] { mCert2 }, "authType"); trustManager1.checkServerTrusted(new X509Certificate[] { mCert1 }, "authType"); } public void testSelfSignedCertificateMatchingHost() throws Exception { mKeyStore.addCertificate(MATCHING_HOST, PORT1, mCert1); - X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1, true); + X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1); trustManager.checkServerTrusted(new X509Certificate[] { mCert1 }, "authType"); } public void testSelfSignedCertificateNotMatchingHost() throws Exception { mKeyStore.addCertificate(NOT_MATCHING_HOST, PORT1, mCert1); - X509TrustManager trustManager = TrustManagerFactory.get(NOT_MATCHING_HOST, PORT1, true); + X509TrustManager trustManager = TrustManagerFactory.get(NOT_MATCHING_HOST, PORT1); trustManager.checkServerTrusted(new X509Certificate[] { mCert1 }, "authType"); } public void testWrongCertificate() throws Exception { mKeyStore.addCertificate(MATCHING_HOST, PORT1, mCert1); - X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1, true); + X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1); assertCertificateRejection(trustManager, new X509Certificate[] { mCert2 }); } @@ -242,44 +242,44 @@ public class TrustManagerFactoryTest extends AndroidTestCase { mKeyStore.addCertificate(MATCHING_HOST, PORT1, mCert1); mKeyStore.addCertificate(MATCHING_HOST, PORT2, mCert2); - X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1, true); + X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1); assertCertificateRejection(trustManager, new X509Certificate[] { mCert2 }); } public void testUntrustedCertificateChain() throws Exception { - X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1, true); + X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1); assertCertificateRejection(trustManager, new X509Certificate[] { mCert3, mCaCert }); } public void testLocallyTrustedCertificateChain() throws Exception { mKeyStore.addCertificate(MATCHING_HOST, PORT1, mCert3); - X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1, true); + X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1); trustManager.checkServerTrusted(new X509Certificate[] { mCert3, mCaCert }, "authType"); } public void testLocallyTrustedCertificateChainNotMatchingHost() throws Exception { mKeyStore.addCertificate(NOT_MATCHING_HOST, PORT1, mCert3); - X509TrustManager trustManager = TrustManagerFactory.get(NOT_MATCHING_HOST, PORT1, true); + X509TrustManager trustManager = TrustManagerFactory.get(NOT_MATCHING_HOST, PORT1); trustManager.checkServerTrusted(new X509Certificate[] { mCert3, mCaCert }, "authType"); } public void testGloballyTrustedCertificateChain() throws Exception { - X509TrustManager trustManager = TrustManagerFactory.get("www.linux.com", PORT1, true); + X509TrustManager trustManager = TrustManagerFactory.get("www.linux.com", PORT1); X509Certificate[] certificates = new X509Certificate[] { mLinuxComCert, mStarfieldCert }; trustManager.checkServerTrusted(certificates, "authType"); } public void testGloballyTrustedCertificateNotMatchingHost() throws Exception { - X509TrustManager trustManager = TrustManagerFactory.get(NOT_MATCHING_HOST, PORT1, true); + X509TrustManager trustManager = TrustManagerFactory.get(NOT_MATCHING_HOST, PORT1); assertCertificateRejection(trustManager, new X509Certificate[] { mLinuxComCert, mStarfieldCert }); } public void testGloballyTrustedCertificateNotMatchingHostOverride() throws Exception { mKeyStore.addCertificate(MATCHING_HOST, PORT1, mLinuxComCert); - X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1, true); + X509TrustManager trustManager = TrustManagerFactory.get(MATCHING_HOST, PORT1); X509Certificate[] certificates = new X509Certificate[] { mLinuxComCert, mStarfieldCert }; trustManager.checkServerTrusted(certificates, "authType"); }