SslHelper has been removed, and its functionality has been transferred
into TrustedSocketFactory. The added layer of indirection wasn't really
simplifying anything. It's now easier to see what happens when
createSocket() is invoked.
A new instance of SecureRandom is no longer passed to SSLContext.init().
Instead, null is passed.
The (default) provider of the TLS SSLContext used is OpenSSLProvider,
which provides an SSLSocket instance of type OpenSSLSocketImpl. The only
use of SecureRandom is in OpenSSLSocketImpl.startHandshake(), where it is
used to seed the OpenSSL PRNG with additional random data. But if
SecureRandom is null, then /dev/urandom is used for seeding instead.
Meanwhile, the default provider for the SecureRandom service is
OpenSSLRandom, which uses the OpenSSL PRNG as its data source. So we were
effectively seeding the OpenSSL PRNG with itself. That's probably okay
(we trust that the OpenSSL PRNG was properly initialized with random data
before first use), but using /dev/urandom would seem like a better source
(or at least as good a source) for the additional seed data added with
each new connection.
Note that our PRNGFixes class replaces the default SecureRandom service
with one whose data source is /dev/urandom for certain vulnerable API
levels anyway. (It also makes sure that the OpenSSL PRNG is properly
seeded before first use for certain vulnerable API levels.)
This was dead code. The exception message will always start with either
"SMTP response is 0 length" from checkLine() or else "Negative SMTP reply"
from NegativeSmtpReplyException().
The problem originated from way back before 4.904.
This is done when the SASL EXTERNAL mechanism isn't advertised (indicating
the possibility that the server did not accept the client certificate) or
when the command for authenticating with SASL EXTERNAL fails.
The CertificateValidationException will trigger a notification to the user
that there's an authentication problem that needs addressing.
Also, there were instances where CertificateValidationException was being
thrown with a new CertificateException as the cause for the purpose of
notifying the user when STARTTLS is not available. This has been slightly
simplified by eliminating the need to include a new CertificateException
as a cause.
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.
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)
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."
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
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.
Jan Berkel