1
0
mirror of https://github.com/moparisthebest/k-9 synced 2024-11-17 23:15:10 -05:00
Commit Graph

648 Commits

Author SHA1 Message Date
Joe Steele
f7fb0cca41 Compare Enum types with ==, !=
Per comments in PR #473

https://github.com/k9mail/k-9/pull/474#commitcomment-7416979

https://github.com/k9mail/k-9/pull/474#commitcomment-7416999
2014-08-18 18:12:39 -04:00
Joe Steele
301ac48a38 Throw CertificateValidationException if EXTERNAL authentication fails
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.
2014-08-11 11:07:54 -04:00
Joe Steele
c0be0eea12 Use the correct POP3 AUTH command 2014-08-11 11:07:51 -04:00
Joe Steele
88016ae52e Revert unused code changes 2014-08-11 11:07:37 -04:00
Dominik Schürmann
aad171ff7e Client Certificate Authentication 2014-06-05 21:03:18 +02:00
cketti
5c93f105ea Avoid NullPointerException reported via Google Play 2014-03-23 00:39:10 +01:00
Joe Steele
95f62785fc Eliminate unused field/parameter 2014-03-20 09:47:43 -04:00
Joe Steele
01d2247ffd Change POP3 error response detection
Instead of interpreting a "-" at the beginning of a line as
an error response, consider the absence of a "+" at the
beginning of a line as an error response.

This is what Thunderbird does.

http://hg.mozilla.org/releases/comm-esr24/file/55e96a433bd1/mailnews/local/src/nsPop3Protocol.cpp#l1177

The problem arises with godaddy servers spewing additional
lines of data upon login failure.  The login was being
interpreted as successful, and a STAT commanded was subsequently
being sent, resulting in a dialog saying 'Cannot connect to
server. (Invalid int: "auth_error:")'.

$ openssl s_client -quiet -crlf -connect pop.secureserver.net:995
...
+OK <24984.1394317012@pop.secureserver.net>
user testuser
+OK
pass testpass
testuser not found in the auth database
warning: auth_error: authorization failed (no such object)
-ERR authorization failed  Check your server settings.
2014-03-11 19:10:14 -04:00
Joe Steele
e475e51731 Rework handling of certificate errors while pushing
Eliminate import of MessagingController in ImapStore.
2014-03-11 19:08:09 -04:00
Joe Steele
a7898fa2eb Fix issue 6269: IMAP LOGIN failure
Some IMAP servers are broken and don't correctly handle string
literals with the LOGIN command.

This switches to using quoted strings instead.

This is what Thunderbird does.
2014-03-11 19:06:00 -04:00
cketti
ff5edf43d4 Merge branch 'pr/453'
Authentication changes

 message to explain why this merge is necessary,
2014-03-05 06:19:55 +01:00
cketti
617123c58b Remove SimpleX509TrustManager because it's no longer used 2014-03-05 06:03:06 +01:00
cketti
0a63466704 Add missing import 2014-03-05 04:16:57 +01:00
Joe Steele
094318dacb Use buffered output streams for all output
POP3 already does this.

This is a more general solution to the problem
addressed in commit 8bfd6ca.
2014-03-03 17:31:26 -05:00
Joe Steele
9dc5338501 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.
2014-03-03 17:29:51 -05:00
Joe Steele
14a0a7a2a7 Provide notification if STARTTLS is not available 2014-03-03 17:29:48 -05:00
Joe Steele
daea7f1ecd 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
<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.
2014-03-03 17:23:00 -05:00
Joe Steele
39590d49bd 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.
2014-03-03 17:18:49 -05:00
Joe Steele
8d0901a178 Delete old journals when moving the database 2014-03-03 11:03:16 -05:00
Joe Steele
0f991b434e Use Locale.US where appropriate
Applicable for strings not intended for user consumption.

A %d string format code can generate eastern-arabic numerals
for users with an arabic locale.

V/k9      (20763): conn1103774136>>> 5 UID SEARCH ٦٤٦:٦٧٠ NOT DELETED
V/k9      (20763): conn1103774136<<<#5# [BAD, Invalid Search criteria]
E/k9      (20763): synchronizeMailbox
E/k9      (20763): com.fsck.k9.mail.store.ImapStore$ImapException: Command: UID SEARCH ٦٤٦:٦٧٠ NOT DELETED; response: #5# [BAD, Invalid Search criteria]
2014-03-03 10:40:23 -05:00
Joe Steele
0509e1541c Use Locale.US where appropriate 2014-03-03 10:08:07 -05:00
cketti
47e09c92ea Merge pull request #456 from asdil12/single_pkg_command
Fix sendCommand line splitup
2014-03-01 07:39:52 +01:00
Dominik Heidler
8bfd6ca3e0 Fix sendCommand line splitup for some imap proxys
When sending a command it would be sent like this:
PKG1: 1 STARTTLS
PKG2: \r\n

Some imap proxys (maybe from Fortinet?) don't accept commands across packets:
PKG1: 1 STARTTLS\r\n
2014-02-28 18:17:11 +01:00
Miltos Allamanis
2df205874e Renamed "s" to "sizeParam".
The naturalize tool detected that using "sizeParam" is more consistent
with the current codebase state:
* "sizeParam" in LocalStore is 22.89% probable ("s" 10.52%)
2014-02-26 14:27:35 +00:00
Miltos Allamanis
be2b3b1ec2 Renamed "usee" to "uee".
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%)
2014-02-26 14:23:33 +00:00
Joe Steele
c7e46faf0a Simplify code with better use of enum ConnectionSecurity 2014-02-25 15:22:38 -05:00
Joe Steele
dc9720ca13 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)
2014-02-25 15:22:35 -05:00
Joe Steele
64fd04ece2 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.
2014-02-25 15:22:32 -05:00
Joe Steele
26491676fa 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."
2014-02-25 15:22:30 -05:00
Joe Steele
3877f58515 (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.)
2014-02-25 15:22:28 -05:00
Joe Steele
1d1b14da21 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.
2014-02-25 15:22:24 -05:00
Joe Steele
871ee1cc6c 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
2014-02-25 15:22:22 -05:00
András Veres-Szentkirályi
ab3044c9fa use Set instead of implementation type 2014-02-15 23:59:24 +01:00
András Veres-Szentkirályi
df75853c64 replaced entrySet + getKey with keySet 2014-02-15 23:59:23 +01:00
András Veres-Szentkirályi
3d327763b5 replaced for with for-each loop 2014-02-15 23:59:23 +01:00
András Veres-Szentkirályi
e75dd7df39 replaced for with for-each loop 2014-02-15 23:59:23 +01:00
cketti
3527930f89 Fix 'endless' loop in ImapFolderPusher
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
2014-02-11 20:17:47 +01:00
Joe Steele
03925fb409 Fix POP3 STLS command
The server response needed to be retrieved.

Thanks to Paul Durrant:
https://groups.google.com/d/msg/k-9-mail/0XHNNMR1TQ4/yExsr7nvJQwJ
2013-12-29 18:47:02 -05:00
cketti
79a5bc9c7e Revert "Make IMAP autoconfig recognize "Draft" as drafts folder"
This reverts commit 453f10128c.
See https://github.com/k9mail/k-9/pull/429
2013-12-17 18:45:57 +01:00
erlendorf
453f10128c Make IMAP autoconfig recognize "Draft" as drafts folder
Yahoo names it "Draft" instead of the more common "Drafts".
2013-12-16 17:14:56 +01:00
Joe Steele
40404c3700 Move some classes out of com.fsck.k9.mail.store
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
2013-12-02 14:07:57 -05:00
Joe Steele
76605f7d86 Extract code into new LocalKeyStore class
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.
2013-12-02 14:04:40 -05:00
cketti
4b57d79acf Only check against the certificate stored for a server, not all of them 2013-11-29 14:06:02 +01:00
cketti
c5c195d243 Add unit tests for TrustManagerFactory 2013-11-29 10:49:52 +01:00
Joe Steele
a4440b4042 Fix inadequate certificate validation
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.
2013-11-23 13:26:57 -05:00
brian m. carlson
1bfb78ee51 Use TrustedSocketFactory for STARTTLS.
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.
2013-11-10 00:27:01 +00:00
cketti
a036e4d2f9 Merge branch 'tls-hardening' 2013-10-29 04:40:06 +01:00
cketti
a97705ffa9 Refactor TrustedSocketFactory 2013-10-29 04:37:52 +01:00
cketti
8f45d76b5c Extract WebDavSocketFactory 2013-10-29 03:42:37 +01:00
András Veres-Szentkirályi
d84ce6ddb9 Hardened TLS cipher suites and versions
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/
2013-10-15 10:16:42 +02:00