If the alias is empty or null, don't bother using KeyChainKeyManager.
If the alias is not empty, confirm that it is associated with a
certificate, otherwise throw a CertificateValidationException
which will notify the user of the problem and ask the user to
check the server settings.
Likewise, the user is notified if the client certificate was
not accepted by the server.
With this commit, KeyChainKeyManager no longer throws the exception and
AccountSetupCheckSettings no longer catches it.
It was being thrown when the server requested a client certificate but no
client certificate alias had been configured for the server.
The code was making the incorrect assumption that the server would only
request a client certificate when such a certificate was *required*.
However, servers can be configured to accept multiple forms of
authentication, including both password authentication and client
certificate authentication. So a server may request a certificate without
requiring it. If a user has not configured a client certificate, then
that should not be treated as an error because the configuration may be
valid and the server may accept it.
The only indication that a certificate is *required* is when a
SSLProtocolException is thrown, caused by a SSLHandshakeException
resulting from a fatal handshake alert message received from the server.
Unfortunately, such a message is fairly generic and only "indicates that
the sender was unable to negotiate an acceptable set of security
parameters given the options available." So there is no definitive way to
know that a client certificate is required.
Also, KeyChainKeyManager.getCertificateChain() and getPrivateKey() no
longer throw IllegalStateException(). These methods are permitted to
return null, and such a response is appropriate if the user has deleted
client certificates from the device. Again, this may or may not cause the
server to abort the connection, depending on whether the server *requires*
a client certificate.
The app's minSdkVersion = 15 (Android 4.0.3, Ice Cream Sandwich MR1),
so there's no need to test the API level.
This also removes '@SuppressLint("TrulyRandom")'. I find no
documentation for it, nor do I find any additional lint errors
with its removal.
Specifically, warn and block them when attempting to configure Client
Certificate Authentication in combination with Connection Security = None.
If this were not made obvious to the user, they might not understand why
they are not permitted to tap "Next".
Also, move the initialization of all view listeners out of onCreate() into
initializeViewListeners() which is then called near the end of onCreate(),
helping to assure that the listeners won't be triggered during the
initialization of views inside onCreate().
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
<account name>" 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.
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.
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)
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
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.)
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
This isn't the best solution. Most of the developers agree that a right-aligned
triangle or arrow would be a better fit for such an indicator. But we currently
don't have the time to write the code to do it.
Personally, I think we should try to get rid of submenus altogether.
