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.
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."
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.)
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.
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.
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
All \r and \n codes have been replaced with <br />, so the patterns in
these replacements don't match anything.
This problem has existed for some time -- since commits 1ea27d7 and
e12dd32.
No attempt is made here to reimplement the replacements because users are
now used to the current behavior without much apparent complaint, and such
replacements are never perfect and can occasionally fail to work as
desired without additional tweaking for special cases.
There's currently a bug in linkifyText() that can lead to a
StringIndexOutOfBoundsException when the text contains a
bitcoin URI and a "web" URI near the end of the text.
The loop extracted keys from `folderMap` and then called
`folderMap.get(...)` for every key. If both the key and the value needs
to be iterated on, `Map.entrySet()` is a more efficient solution as it
doesn't require O(n) Map lookups.
Since id is a Long, Long.valueOf(long) unboxed the Long to a primitive
long, then reboxed it into a Long instance, which was again unboxed to
allow it to be set as an element of the array of longs. This commit
reduces the number of boxings from 3 to 1.
Under certain circumstances it's possible that the 'push state' isn't
updated to contain the most recent 'UIDNEXT' value. In that case
ImapFolderPusher.start() would execute the same code path through its
main loop over and over again, preventing the device from going to
sleep.
Rather than changing the code to update the 'push state' in the corner
case that triggers the behavior described above, this commit introduces
another mechanism to track the 'UIDNEXT' value. This should also catch
as of yet unknown cases where the 'push state' isn't properly updated.
At some point in the future I hope we get to a point where we only
persist the 'push state' when we manually stop/restart the service.
During normal operation there's no need to read from/write to storage
all the time.
Fixes issue 4907
When message viewing and tap the next icon, menu icons (previous, next, delete and replys) disappears for an instant.
But a compose icon remains, then tap it accidentally.
Eliminate the invocation of
WebSettings.setBlockNetworkImage(boolean flag),
thus maintaining the the default setting of "false".
On Android versions prior to KitKat, this setting has no
effect on inline image attachments loaded with content:
URIs. Such images would load regardless.
With KitKat, this setting does have an effect -- a
setting of "true" will block image attachments loaded
with content: URIs.
By removing this call, K-9 Mail behaves the same on KitKat
as on earlier Android versions, and the behavior on earlier
versions is unchanged.
The minSdkVersion was recently increased from 8 to 15.
WebSettings.setBlockNetworkLoads has been publicly available
since API level 8 (Froyo).
StrictMode has been publicly available since API level 9
(Gingerbread).
Also, include the sent-date in the header when using
the "prefix" quote style. "Be like mutt" (and gmail,
and thunderbird)
Also, the quoteOriginalHtmlMessage method was using the mSourceMessage
field in various places when it should be using its originalMessage
parameter.
Related issues: 2249, 3456
There were a number of preferences that were not being removed
from the preferences DB when an account was deleted, so they
would remain there forever.
There were a few attempts to remove preference keys from the DB where
the keys were obsolete and not in use for some time.
Certain obsolete preferences were not modified:
mUuid + ".name"
mUuid + ".email"
mUuid + ".signature"
mUuid + ".signatureUse"
These were in use before implementing multiple identities, and are still used
as a fallback for old accounts without multiple identities configured.
The error reporting assures an exception is thrown if
setKeyStoreFile(null) is called without a prior call to
setKeyStoreLocation(String directory).
Also, fix TrustManagerFactoryTest indentation.
Blacklist a couple of weak ciphers, bring known ones in a defined order and sort unknown
ciphers at the end. Also re-enable SSLv3 because it's still used a lot.
Instead, have K9.onCreate initialize the location of the key
store file (similar to what is done with
BinaryTempFileBody.setTempDirectory).
Also, LocalKeyStore.getInstance has been changed so that it
no longer needs to be synchronized.
Implement an "upgrade" capability for the key store file,
and then use it to delete the old file.
The existing certs in the old file are not a security
risk, but they are now useless because the format of
their aliases was changed in commit a4440b4. They now are
just taking up storage space and memory.
Users will need to re-accept *ALL* certificates that they had
previously accepted and are still using. (Actually, this requirement
was effective with commit 4b57d79a. Before that, certificates whose
Subject matched did not require re-accepting.)
The classes are just as much related to com.fsck.k9.mail.transport
as com.fsck.k9.mail.store, so having them in
com.fsck.k9.mail.store doesn't seem appropriate.
Move LocalKeyStore to com.fsck.k9.security
Move TrustManagerFactory and TrustedSocketFactory to com.fsck.k9.net.ssl
Also, implement the ability to configure an alternate key store
file location. This permits the running of unit tests without
clobbering the live key store file.
Also, add a test to confirm that the key store file is being written
out and reread correctly.
Proper host name validation was not being performed for certificates
kept in the local keystore. If an attacker could convince a user to
accept and store an attacker's certificate, then that certificate
could be used for MITM attacks, giving the attacker access to all
connections to all servers in all accounts in K-9.
This commit changes how the certificates are stored. Previously, an
entire certificate chain was stored for a server (and any of those
certificates in the chain were available for validating signatures on
certificates received when connecting). Now just the single
certificate for the server is stored.
This commit changes how locally stored certificates are retrieved.
They can only be retrieved using the host:port that the user
configured for the server.
This also fixes issue 1326. Users can now use different certificates
for different servers on the same host (listening to different ports).
The above changes mean that users might have to re-accept certificates
that they had previously accepted and are still using (but only if the
certificate's Subject doesn't match the host that they are connecting
to).
This commit modifies AccountSetupBasics so that it now calls
AccountSetupCheckSettings twice -- once for checking the incoming
settings and once for the outgoing settings. Otherwise, an exception
could occur while checking incoming settings, the user could say
continue (or the user could accept a certificate key), and the
outgoing settings would not be checked. This also helps with
determining if a certificate exception was for the incoming or
outgoing server, which is needed if the user decides to add the
certificate to the keystore.
The TrustedSocketFactory, which provides goodies like better cipher suites and
TLSv1.2, was only being used for tunnelled connections. Use it for STARTTLS
connections as well.
Implement methods missing in API 7
Fix coordinate reference frame mismatch. Touch events are
relative to the view display, whereas view children are relative
to the view.
Revert "Only Jelly Bean seems to have the auto-scroll issue"
This reverts commit a3802a7a8e.
Revert "Hack around WebView's initial auto-scrolling past the message header"
This reverts commit 8dcc769c50.
Conflicts:
src/com/fsck/k9/view/MessageWebView.java
This implements the AOSP Email solution for incorporating
a Webview inside a ScrollView, while still being able to
scroll diagonally.
This replaces the functionality of TitleBarWebView (which
is now removed).
As Georg Lukas wrote in his blog post about how Android handles TLS
handshake (http://op-co.de/blog/posts/android_ssl_downgrade/), an
explicit order of cipher suites and TLS versions must be supplied to
avoid having the weak (presumably broken) RC4 cipher at the top of the
preference list.
This commit adds the list included in the blog post to every TLS socket
creation, including IMAP, POP3 and SMTP, see Wireshark screenshots done
during testing at http://vsza.hu/k9mail-tls-hardening/
This string resource is used in two places -- both with and without the
linefeed at the end. Instead of having a linefeed in the string and
having the code remove it if not needed, the linefeed is now omitted from
the string and the code adds it if needed.
Also, the line ending is changed from \n to \r\n.
Also, the string in the DE and FR locales had linefeeds at the start that
were removed so they match all the other locales.
(The string in the zh-rTW locale was left alone, since it had no
linefeeds. It looks like that file has numerous instances where \n was
replaced with actual newlines, which is probably not correct.)
This builds upon the efforts started 2 commits back where \r\n is used for
all message text and \n is only used when the text is inside an
EolConvertingEditText widget.
Even with the fix in the parent commit, the X-K9mail-Identity header can
become invalid if, for example, a user creates a draft in K-9 Mail, then
edits the draft outside of K-9 Mail, then opens the draft again in K-9
Mail.
This commit assures that an invalid X-K9mail-Identity header will not
result in an IndexOutOfBoundsException.
The problem:
Configure the account (just an example -- problems can occur in other
configurations as well):
Message Format: HTML
Reply quoting style: Prefix
Quote message when replying: yes
Reply after quoted text: yes
Reply to a message that has a large quantity (20+) of \r\n scattered in
the body of its HTML version (not an unusual scenario).
Add a reply. Save the message as a draft. Go back & open the draft
again. A fatal IndexOutOfBoundsException occurs.
The cause:
When the draft was saved, the X-K9mail-Identity header was computed and
added to the message, then the text of the message was processed with
MimeUtility.fixDraftTextBody, replacing all occurrences of \r\n with \n in
the quoted message before being saved in LocalStore, thus invalidating the
X-K9mail-Identity header.
The fix:
Remove MimeUtility.fixDraftTextBody and implement
MessageCompose$EolConvertingEditText instead. Any message text placed in
an EolConvertingEditText widget is assured to have \n line endings. Any
message text extracted from an EolConvertingEditText widget is assured to
have \r\n line endings. The X-K9mail-Identity header will always be
computed correctly.
Issues thought to be related: 4782, 5010, 5634, 5725
As noted in some of the referenced issues, errors didn't always result in
a fatal exception, but instead with mixed up text.
Ref: commit f9a35aeaee
