moparisthebest/k-9
1
0
Fork 0
mirror of https://github.com/moparisthebest/k-9 synced 2024-12-24 08:38:51 -05:00
Code Issues Releases Wiki Activity

Fix inadequate certificate validation

Browse Source 
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.
This commit is contained in:
Joe Steele 2013-11-23 13:26:57 -05:00
parent b500047e42
commit a4440b4042
11 changed files with 93 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/com/fsck/k9/activity/setup/AccountSetupBasics.java
View File

@ -17,6 +17,7 @@ import android.widget.Button;
import android.widget.EditText;
import com.fsck.k9.*;
import com.fsck.k9.activity.K9Activity;
import com.fsck.k9.activity.setup.AccountSetupCheckSettings.CheckDirection;
import com.fsck.k9.helper.Utility;
import java.io.Serializable;
import java.io.UnsupportedEncodingException;
@ -46,6 +47,7 @@ public class AccountSetupBasics extends K9Activity
private Provider mProvider;
private EmailAddressValidator mEmailValidator = new EmailAddressValidator();
private boolean mCheckedIncoming = false;
public static void actionNewAccount(Context context) {
Intent i = new Intent(context, AccountSetupBasics.class);
@ -229,7 +231,8 @@ public class AccountSetupBasics extends K9Activity
} else if (incomingUri.toString().startsWith("pop3")) {
mAccount.setDeletePolicy(Account.DELETE_POLICY_NEVER);
}
AccountSetupCheckSettings.actionCheckSettings(this, mAccount, true, true);
// Check incoming here. Then check outgoing in onActivityResult()
AccountSetupCheckSettings.actionCheckSettings(this, mAccount, CheckDirection.INCOMING);
} catch (UnsupportedEncodingException enc) {
// This really shouldn't happen since the encoding is hardcoded to UTF-8
Log.e(K9.LOG_TAG, "Couldn't urlencode username or password.", enc);
@ -266,11 +269,18 @@ public class AccountSetupBasics extends K9Activity
@Override
public void onActivityResult(int requestCode, int resultCode, Intent data) {
if (resultCode == RESULT_OK) {
mAccount.setDescription(mAccount.getEmail());
mAccount.save(Preferences.getPreferences(this));
K9.setServicesEnabled(this);
AccountSetupNames.actionSetNames(this, mAccount);
finish();
if (!mCheckedIncoming) {
//We've successfully checked incoming. Now check outgoing.
mCheckedIncoming = true;
AccountSetupCheckSettings.actionCheckSettings(this, mAccount, CheckDirection.OUTGOING);
} else {
//We've successfully checked outgoing as well.
mAccount.setDescription(mAccount.getEmail());
mAccount.save(Preferences.getPreferences(this));
K9.setServicesEnabled(this);
AccountSetupNames.actionSetNames(this, mAccount);
finish();
}
}
}

41
src/com/fsck/k9/activity/setup/AccountSetupCheckSettings.java
View File

@ -47,9 +47,12 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
private static final String EXTRA_ACCOUNT = "account";
private static final String EXTRA_CHECK_INCOMING = "checkIncoming";
private static final String EXTRA_CHECK_DIRECTION ="checkDirection";
private static final String EXTRA_CHECK_OUTGOING = "checkOutgoing";
public enum CheckDirection {
INCOMING,
OUTGOING
}
private Handler mHandler = new Handler();
@ -59,20 +62,16 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
private Account mAccount;
private boolean mCheckIncoming;
private boolean mCheckOutgoing;
private CheckDirection mDirection;
private boolean mCanceled;
private boolean mDestroyed;
public static void actionCheckSettings(Activity context, Account account,
boolean checkIncoming, boolean checkOutgoing) {
public static void actionCheckSettings(Activity context, Account account, CheckDirection direction) {
Intent i = new Intent(context, AccountSetupCheckSettings.class);
i.putExtra(EXTRA_ACCOUNT, account.getUuid());
i.putExtra(EXTRA_CHECK_INCOMING, checkIncoming);
i.putExtra(EXTRA_CHECK_OUTGOING, checkOutgoing);
i.putExtra(EXTRA_CHECK_DIRECTION, direction);
context.startActivityForResult(i, ACTIVITY_REQUEST_CODE);
}
@ -89,8 +88,7 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
String accountUuid = getIntent().getStringExtra(EXTRA_ACCOUNT);
mAccount = Preferences.getPreferences(this).getAccount(accountUuid);
mCheckIncoming = getIntent().getBooleanExtra(EXTRA_CHECK_INCOMING, false);
mCheckOutgoing = getIntent().getBooleanExtra(EXTRA_CHECK_OUTGOING, false);
mDirection = (CheckDirection) getIntent().getSerializableExtra(EXTRA_CHECK_DIRECTION);
new Thread() {
@Override
@ -108,9 +106,9 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
final MessagingController ctrl = MessagingController.getInstance(getApplication());
ctrl.clearCertificateErrorNotifications(AccountSetupCheckSettings.this,
mAccount, mCheckIncoming, mCheckOutgoing);
mAccount, mDirection);
if (mCheckIncoming) {
if (mDirection.equals(CheckDirection.INCOMING)) {
store = mAccount.getRemoteStore();
if (store instanceof WebDavStore) {
@ -133,7 +131,7 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
finish();
return;
}
if (mCheckOutgoing) {
if (mDirection.equals(CheckDirection.OUTGOING)) {
if (!(mAccount.getRemoteStore() instanceof WebDavStore)) {
setMessage(R.string.account_setup_check_settings_check_outgoing_msg);
}
@ -366,21 +364,20 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
new DialogInterface.OnClickListener() {
public void onClick(DialogInterface dialog, int which) {
try {
String alias = mAccount.getUuid();
if (mCheckIncoming) {
alias = alias + ".incoming";
Uri uri = null;
if (mDirection.equals(CheckDirection.INCOMING)) {
uri = Uri.parse(mAccount.getStoreUri());
} else {
uri = Uri.parse(mAccount.getTransportUri());
}
if (mCheckOutgoing) {
alias = alias + ".outgoing";
}
TrustManagerFactory.addCertificateChain(alias, chain);
TrustManagerFactory.addCertificate(uri.getHost(), uri.getPort(), chain[0]);
} catch (CertificateException e) {
showErrorDialog(
R.string.account_setup_failed_dlg_certificate_message_fmt,
e.getMessage() == null ? "" : e.getMessage());
}
AccountSetupCheckSettings.actionCheckSettings(AccountSetupCheckSettings.this, mAccount,
mCheckIncoming, mCheckOutgoing);
mDirection);
}
})
.setNegativeButton(

3
src/com/fsck/k9/activity/setup/AccountSetupIncoming.java
View File

@ -16,6 +16,7 @@ import android.widget.CompoundButton.OnCheckedChangeListener;
import com.fsck.k9.*;
import com.fsck.k9.activity.K9Activity;
import com.fsck.k9.activity.setup.AccountSetupCheckSettings.CheckDirection;
import com.fsck.k9.helper.Utility;
import com.fsck.k9.mail.ConnectionSecurity;
import com.fsck.k9.mail.ServerSettings;
@ -437,7 +438,7 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener
mAccount.setCompression(Account.TYPE_OTHER, mCompressionOther.isChecked());
mAccount.setSubscribedFoldersOnly(mSubscribedFoldersOnly.isChecked());
AccountSetupCheckSettings.actionCheckSettings(this, mAccount, true, false);
AccountSetupCheckSettings.actionCheckSettings(this, mAccount, CheckDirection.INCOMING);
} catch (Exception e) {
failure(e);
}

5
src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java
View File

@ -15,6 +15,7 @@ import android.widget.*;
import android.widget.CompoundButton.OnCheckedChangeListener;
import com.fsck.k9.*;
import com.fsck.k9.activity.K9Activity;
import com.fsck.k9.activity.setup.AccountSetupCheckSettings.CheckDirection;
import com.fsck.k9.helper.Utility;
import com.fsck.k9.mail.transport.SmtpTransport;
@ -95,7 +96,7 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener,
try {
if (new URI(mAccount.getStoreUri()).getScheme().startsWith("webdav")) {
mAccount.setTransportUri(mAccount.getStoreUri());
AccountSetupCheckSettings.actionCheckSettings(this, mAccount, false, true);
AccountSetupCheckSettings.actionCheckSettings(this, mAccount, CheckDirection.OUTGOING);
}
} catch (URISyntaxException e) {
// TODO Auto-generated catch block
@ -311,7 +312,7 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener,
uri = new URI(smtpSchemes[securityType], userInfo, mServerView.getText().toString(),
Integer.parseInt(mPortView.getText().toString()), null, null, null);
mAccount.setTransportUri(uri.toString());
AccountSetupCheckSettings.actionCheckSettings(this, mAccount, false, true);
AccountSetupCheckSettings.actionCheckSettings(this, mAccount, CheckDirection.OUTGOING);
} catch (UnsupportedEncodingException enc) {
// This really shouldn't happen since the encoding is hardcoded to UTF-8
Log.e(K9.LOG_TAG, "Couldn't urlencode username or password.", enc);

8
src/com/fsck/k9/controller/MessagingController.java
View File

@ -53,6 +53,7 @@ import com.fsck.k9.activity.FolderList;
import com.fsck.k9.activity.MessageList;
import com.fsck.k9.activity.MessageReference;
import com.fsck.k9.activity.NotificationDeleteConfirmation;
import com.fsck.k9.activity.setup.AccountSetupCheckSettings.CheckDirection;
import com.fsck.k9.activity.setup.AccountSetupIncoming;
import com.fsck.k9.activity.setup.AccountSetupOutgoing;
import com.fsck.k9.cache.EmailProviderCache;
@ -2671,14 +2672,13 @@ public class MessagingController implements Runnable {
}
public void clearCertificateErrorNotifications(Context context,
final Account account, boolean incoming, boolean outgoing) {
final Account account, CheckDirection direction) {
final NotificationManager nm = (NotificationManager)
context.getSystemService(Context.NOTIFICATION_SERVICE);
if (incoming) {
if (direction.equals(CheckDirection.INCOMING)) {
nm.cancel(null, K9.CERTIFICATE_EXCEPTION_NOTIFICATION_INCOMING + account.getAccountNumber());
}
if (outgoing) {
} else {
nm.cancel(null, K9.CERTIFICATE_EXCEPTION_NOTIFICATION_OUTGOING + account.getAccountNumber());
}
}

17
src/com/fsck/k9/mail/store/ImapStore.java
View File

@ -2446,9 +2446,12 @@ public class ImapStore extends Store {
connectionSecurity == CONNECTION_SECURITY_SSL_OPTIONAL) {
SSLContext sslContext = SSLContext.getInstance("TLS");
boolean secure = connectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED;
sslContext.init(null, new TrustManager[] {
TrustManagerFactory.get(mSettings.getHost(), secure)
}, new SecureRandom());
sslContext
.init(null,
new TrustManager[] { TrustManagerFactory.get(
mSettings.getHost(),
mSettings.getPort(), secure) },
new SecureRandom());
mSocket = TrustedSocketFactory.createSocket(sslContext);
} else {
mSocket = new Socket();
@ -2501,9 +2504,11 @@ public class ImapStore extends Store {
SSLContext sslContext = SSLContext.getInstance("TLS");
boolean secure = mSettings.getConnectionSecurity() == CONNECTION_SECURITY_TLS_REQUIRED;
sslContext.init(null, new TrustManager[] {
TrustManagerFactory.get(mSettings.getHost(), secure)
}, new SecureRandom());
sslContext.init(null,
new TrustManager[] { TrustManagerFactory.get(
mSettings.getHost(),
mSettings.getPort(), secure) },
new SecureRandom());
mSocket = TrustedSocketFactory.createSocket(sslContext, mSocket,
mSettings.getHost(), mSettings.getPort(), true);
mSocket.setSoTimeout(Store.SOCKET_READ_TIMEOUT);

13
src/com/fsck/k9/mail/store/Pop3Store.java
View File

@ -327,9 +327,9 @@ public class Pop3Store extends Store {
mConnectionSecurity == CONNECTION_SECURITY_SSL_OPTIONAL) {
SSLContext sslContext = SSLContext.getInstance("TLS");
final boolean secure = mConnectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED;
sslContext.init(null, new TrustManager[] {
TrustManagerFactory.get(mHost, secure)
}, new SecureRandom());
sslContext.init(null,
new TrustManager[] { TrustManagerFactory.get(mHost,
mPort, secure) }, new SecureRandom());
mSocket = TrustedSocketFactory.createSocket(sslContext);
} else {
mSocket = new Socket();
@ -356,9 +356,10 @@ public class Pop3Store extends Store {
SSLContext sslContext = SSLContext.getInstance("TLS");
boolean secure = mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED;
sslContext.init(null, new TrustManager[] {
TrustManagerFactory.get(mHost, secure)
}, new SecureRandom());
sslContext.init(null,
new TrustManager[] { TrustManagerFactory.get(
mHost, mPort, secure) },
new SecureRandom());
mSocket = TrustedSocketFactory.createSocket(sslContext, mSocket, mHost,
mPort, true);
mSocket.setSoTimeout(Store.SOCKET_READ_TIMEOUT);

36
src/com/fsck/k9/mail/store/TrustManagerFactory.java
View File

@ -18,6 +18,7 @@ import java.io.IOException;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.security.cert.Certificate;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.HashMap;
@ -53,18 +54,21 @@ public final class TrustManagerFactory {
new HashMap<String, SecureX509TrustManager>();
private final String mHost;
private final int mPort;
private SecureX509TrustManager(String host) {
private SecureX509TrustManager(String host, int port) {
mHost = host;
mPort = port;
}
public synchronized static X509TrustManager getInstance(String host) {
public synchronized static X509TrustManager getInstance(String host, int port) {
String key = getCertKey(host, port);
SecureX509TrustManager trustManager;
if (mTrustManager.containsKey(host)) {
trustManager = mTrustManager.get(host);
if (mTrustManager.containsKey(key)) {
trustManager = mTrustManager.get(key);
} else {
trustManager = new SecureX509TrustManager(host);
mTrustManager.put(host, trustManager);
trustManager = new SecureX509TrustManager(host, port);
mTrustManager.put(key, trustManager);
}
return trustManager;
@ -89,8 +93,9 @@ public final class TrustManagerFactory {
}
if (!DomainNameChecker.match(chain[0], mHost)) {
try {
String dn = chain[0].getSubjectDN().toString();
if ((dn != null) && (dn.equalsIgnoreCase(keyStore.getCertificateAlias(chain[0])))) {
Certificate storedCert = keyStore
.getCertificate(getCertKey(mHost, mPort));
if (storedCert != null && storedCert.equals(chain[0])) {
return;
}
} catch (KeyStoreException e) {
@ -164,8 +169,8 @@ public final class TrustManagerFactory {
private TrustManagerFactory() {
}
public static X509TrustManager get(String host, boolean secure) {
return secure ? SecureX509TrustManager.getInstance(host) :
public static X509TrustManager get(String host, int port, boolean secure) {
return secure ? SecureX509TrustManager.getInstance(host, port) :
unsecureTrustManager;
}
@ -173,13 +178,10 @@ public final class TrustManagerFactory {
return keyStore;
}
public static void addCertificateChain(String alias, X509Certificate[] chain) throws CertificateException {
public static void addCertificate(String host, int port, X509Certificate certificate) throws CertificateException {
try {
javax.net.ssl.TrustManagerFactory tmf = javax.net.ssl.TrustManagerFactory.getInstance("X509");
for (X509Certificate element : chain) {
keyStore.setCertificateEntry
(element.getSubjectDN().toString(), element);
}
keyStore.setCertificateEntry(getCertKey(host, port), certificate);
tmf.init(keyStore);
TrustManager[] tms = tmf.getTrustManagers();
@ -211,4 +213,8 @@ public final class TrustManagerFactory {
Log.e(LOG_TAG, "Key Store exception while initializing TrustManagerFactory ", e);
}
}
private static String getCertKey(String host, int port) {
return host + ":" + port;
}
}

4
src/com/fsck/k9/mail/store/WebDavSocketFactory.java
View File

@ -26,10 +26,10 @@ public class WebDavSocketFactory implements LayeredSocketFactory {
private SSLSocketFactory mSocketFactory;
private org.apache.http.conn.ssl.SSLSocketFactory mSchemeSocketFactory;
public WebDavSocketFactory(String host, boolean secure) throws NoSuchAlgorithmException, KeyManagementException {
public WebDavSocketFactory(String host, int port, boolean secure) throws NoSuchAlgorithmException, KeyManagementException {
SSLContext sslContext = SSLContext.getInstance("TLS");
sslContext.init(null, new TrustManager[] {
TrustManagerFactory.get(host, secure)
TrustManagerFactory.get(host, port, secure)
}, new SecureRandom());
mSocketFactory = sslContext.getSocketFactory();
mSchemeSocketFactory = org.apache.http.conn.ssl.SSLSocketFactory.getSocketFactory();

2
src/com/fsck/k9/mail/store/WebDavStore.java
View File

@ -1079,7 +1079,7 @@ public class WebDavStore extends Store {
SchemeRegistry reg = mHttpClient.getConnectionManager().getSchemeRegistry();
try {
Scheme s = new Scheme("https", new WebDavSocketFactory(mHost, mSecure), 443);
Scheme s = new Scheme("https", new WebDavSocketFactory(mHost, 443, mSecure), 443);
reg.register(s);
} catch (NoSuchAlgorithmException nsa) {
Log.e(K9.LOG_TAG, "NoSuchAlgorithmException in getHttpClient: " + nsa);

13
src/com/fsck/k9/mail/transport/SmtpTransport.java
View File

@ -242,9 +242,10 @@ public class SmtpTransport extends Transport {
mConnectionSecurity == CONNECTION_SECURITY_SSL_OPTIONAL) {
SSLContext sslContext = SSLContext.getInstance("TLS");
boolean secure = mConnectionSecurity == CONNECTION_SECURITY_SSL_REQUIRED;
sslContext.init(null, new TrustManager[] {
TrustManagerFactory.get(mHost, secure)
}, new SecureRandom());
sslContext.init(null,
new TrustManager[] { TrustManagerFactory.get(
mHost, mPort, secure) },
new SecureRandom());
mSocket = TrustedSocketFactory.createSocket(sslContext);
mSocket.connect(socketAddress, SOCKET_CONNECT_TIMEOUT);
} else {
@ -301,9 +302,9 @@ public class SmtpTransport extends Transport {
SSLContext sslContext = SSLContext.getInstance("TLS");
boolean secure = mConnectionSecurity == CONNECTION_SECURITY_TLS_REQUIRED;
sslContext.init(null, new TrustManager[] {
TrustManagerFactory.get(mHost, secure)
}, new SecureRandom());
sslContext.init(null,
new TrustManager[] { TrustManagerFactory.get(mHost,
mPort, secure) }, new SecureRandom());
mSocket = TrustedSocketFactory.createSocket(sslContext, mSocket, mHost,
mPort, true);
mIn = new PeekableInputStream(new BufferedInputStream(mSocket.getInputStream(),