The naturalize tool detected that using "uee" is more consistent with
the current codebase state:
* "uee" in LocalStore is 28.47% probable ("usee" 5.01%)
* "uee" in TextBody is 45.02% probable ("usee" 9.10%)
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.
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.
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.
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)
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
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
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.
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 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.
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
Fixes assignment problems for emails sent by some issue tracking
systems, which send out mails with a fixed mail address on behalf of
different people.
Update LocalStore code to handle the newly introduced temporary files
for attachments
Conflicts:
res/values/strings.xml
src/com/fsck/k9/activity/MessageCompose.java
Android allows other apps to access protected content of an app without requesting the
necessary permission when the app returns an Intent with FLAG_GRANT_READ_URI_PERMISSION.
This regularly happens as a result of ACTION_GET_CONTENT, i.e. what we use to pick content
to be attached to a message. Accessing that content only works while the receiving activity
is running. Afterwards accessing the content throws a SecurityException because of the
missing permission.
This commit changes K-9 Mail's behavior to copy the content to a temporary file in K-9's
cache directory while the activity is still running.
Fixes issue 4847, 5821
This also fixes bugs related to the fact that K-9 Mail didn't save a copy of attached content
in the message database.
Fixes issue 1187, 3330, 4930
Don't convert the content-type to lower case in
MimeMessage.getContentType. The content-type may have optional parameters
that are case sensitive (boundary, name).
In removing the lower-case conversion from getContentType, a review was
made for inappropriate case-sensitive comparisons which use data obtained
with getContentType. The only ones found were in isMimeType in both
Message and MimeBodyPart.
Case-sensitive instances of isMimeType were made case-insensitive. Also,
isMimeType was moved from Message to MimeMessage for symmetry with
MimeBodyPart (MimeMessage & MimeBodyPart are similar and contain a good
bit of duplication such as this).
The unit test required fixing now that the case of the boundary text is
preserved.
References:
Commits 2c5186 and dc4002 added the toLowerCase to getContentType in
MimeMessage & MimeBodyPart (Issue 94).
Later, commit 50cd60 removed the toLowerCase addition from MimeBodyPart
(Issue 1289).
Fix the unit test to match.
All line endings in the unit test are now the same.
(Just for consistency. Not a big deal, since such problems are fixed when
the messages are run through EOLConvertingOutputStream.)
The preceding commit resulted in attachments of type message/rfc822 being
sent with 8bit encoding even when the SMTP server did not support
8BITMIME. This commit assures that messages will be converted to 7bit
when necessary.
A new interface CompositeBody was created that extends Body, and classes
Message and Multipart were changed from implementing Body to
CompositeBody. Additional classes BinaryTempFileMessageBody and
LocalAttachmentMessageBody were created (by extending BinaryTempFileBody
and LocalAttachmentBody, respectively), and they too implement
CompositeBody.
A CompositeBody is a Body containing a composite-type that can contain
subparts that may require recursive processing when converting from 8bit
to 7bit. The Part to which a CompositeBody belongs is only permitted to
use 8bit or 7bit encoding for the CompositeBody.
Previously, a Message was created so that it was 7bit clean by default
(even though that meant base64 encoding all attachments, including
messages). Then, if the SMTP server supported 8BITMIME,
Message.setEncoding("8bit") was called so that bodies of type TextBody
would been transmitted using 8bit encoding rather than quoted-printable.
Now, messages are created with 8bit encoding by default. Then, if the
SMTP server does not support 8BITMIME, Message.setUsing7bitTransport is
called to recursively convert the message and its subparts to 7bit. The
method setUsing7bitTransport was added to the interfaces Part and
CompositeBody.
setEncoding no longer iterates over parts in Multipart. That task belongs
to setUsing7bitTransport, which may in turn call setEncoding on the parts.
MimeUtility.getEncodingforType was created as a helper function for
choosing a default encoding that should be used for a given MIME type when
an attachment is added to a message (either while composing or when
retrieving from LocalStore).
setEncoding was implemented in MimeBodyPart to assure that the encoding
set in the Part's headers was the same as set for the Part's Body. (The
method already existed in MimeMessage, which has similarities with
MimeBodyPart.)
MimeMessage.parse(InputStream in, boolean recurse) was implemented so that
the parser could be told to recursively process nested messages read from
the InputStream, thus giving access to all subparts at any level that may
need to be converted from 8bit to 7bit.
The problem: Receive a message with an attachment of type message/rfc822
and forward it. When the message is sent, K-9 Mail uses base64 encoding
for the attachment. (Alternatively, you could compose a new message and
add such an attachment from a file using a filing-picking app, but that is
not 100% effective because the app may not choose the correct
message/rfc822 MIME type for the attachment.)
Such encoding is prohibited per RFC 2046 (5.2.1) and RFC 2045 (6.4). Only
8bit or 7bit encoding is permitted for attachments of type message/rfc822.
Thunderbird refuses to decode such attachments. All that is shown is the
base64 encoded body.
This commit implements LocalAttachmentBody.setEncoding. If an attachment
to a newly composed message is itself a message, then setEncoding("8bit")
is called, otherwise setEncoding("base64") is called for the attachment.
Similar behavior occurs when an attachment is retrieved from LocalStore.
The setEncoding method was added to the Body interface, since all
implementations of Body now declare the method.
The problem here differs from that in the preceding commit: Here, the
encoding problem occurs on sending, not on receipt. Here, the entire
message (headers and body) is base64 encoded, not just the body. Here,
the headers correctly identify the encoding used; it's just that the RFC
does not permit such encoding of attached messages. The problem here
could in fact occur in combination with the preceding problem.
Issue 5734 exemplifies the problem: receive a message with an attachment
of type message/rfc822 that doesn't use base64 encoding for the body of
the attached message. K-9 Mail incorrectly stores the attached message
locally with its original headers but using base64 encoding for the body.
A discrepancy thus exists between what the headers say about the encoding
of the body versus the actual encoding used. This is obvious when
attempting to view the attachment (either by using a compatible message
viewer available on the device or by saving the attachment to a file and
viewing the file contents).
The process: When a message with an attached sub-message is received,
Message.parse puts the attachment in a new MimeMessage with the
attachment's body in a BinaryTempFileBody. LocalFolder.saveAttachment
then calls Message.writeTo (which later calls BinaryTempFileBody.writeTo)
to place the entire attachment (headers and body) in a new file that will
become a LocalAttachmentBody. Until now, BinaryTempFileBody.writeTo
could only save the message body using base64 encoding.
This commit implements BinaryTempFileBody.setEncoding and assures that the
body is written out with the same encoding that was found in its headers.
Currently, K-9 Mail detects if an SMTP server supports 8BITMIME (RFC
6152), and if so, TextBody parts are sent with content-transfer-ecoding =
8bit. Otherwise, they are sent using quoted-printable.
This adds the required "BODY=8BITMIME" parameter to the MAIL command when
sending messages to servers that support 8BITMIME.
If you attempted to use SSL to connect to a server that speaks
STARTTLS, you should get an SSL protocol error. Instead, you
were likely to get an "Unrecognized Certificate" error that shows
you an unrelated certificate chain and asks you to accept it or
reject it. Neither action would work because the actual problem
had nothing to do with certificates. The unrelated certificate
chain that popped up had been statically stored when validating
a prior connection to a different server.
With this patch, certificate chains are no longer stored statically
when validating server connections.
Issue 5886 is an example of a user experiencing this problem.
Previously the app crashed when upgrading the database failed. Now we
reset the database version and run the upgrade code again (recreating
all tables).
This reverts commit bbdec62e37.
Aside from being the incorrect solution for fixing the problem
described in pull request 211, the patch generates 'Dead code'
warnings inside the if(){} statements on lines 46 and 47.
The correct fix for the problem was already implemented in commit
5678786c97.
Although the logcat in the pull request was generated after the fix,
line numbers in the log indicate that it was based on an outdated
version of MimeUtility.java from before the fix.
This requires another database schema change. With this change messages
at the root of a thread reference themselves in the 'threads' table,
i.e. 'root' contains the value of 'id' for these messages. It makes
selecting all messages in a thread much simpler.
Previously, <html>, <head>, & <body> tags were
attached to messages before they were stored locally.
But now that the <head> element also needs to include
a <meta> element (for proper MessageWebView display),
it seems unecesary to store all these tags with each
message.
Now the tags are no longer stored with the messages. Instead,
MessageWebView applies the tags before displaying the message.
This also eliminates the need to upgrade an older
message database where all the old messages would have
otherwise needed to be wrapped with the new tags.
Database updates can be surprisingly slow. This lead to slow updates of
the user interface which in turn made working with K-9 Mail not as fun
as it should be. This commit hopefully changes that.
The commit that introduced those notifications also introduced a rather
... interesting design pattern: The CertificateValidationException
notified the user of its pure existance - it's no longer a 'message'
only, but defines policy. As this is more than unusual, replace this
pattern by the MessagingController treating
CertificateValidationException specially when accessing remote folders.
Also make clear which account failed when constructing the notification.
