From 8eef43c282909eaf8dd75161a73cd66a5e2fc31e Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Mon, 2 Dec 2013 14:33:01 -0500 Subject: [PATCH] Implement pruning of old certificates from LocalKeyStore Certificates are deleted whenever server settings are changed or an account is deleted. --- src/com/fsck/k9/Preferences.java | 2 + .../setup/AccountSetupCheckSettings.java | 8 +- .../activity/setup/AccountSetupIncoming.java | 2 + .../activity/setup/AccountSetupOutgoing.java | 7 +- src/com/fsck/k9/security/LocalKeyStore.java | 77 ++++++++++++++++++- 5 files changed, 84 insertions(+), 12 deletions(-) diff --git a/src/com/fsck/k9/Preferences.java b/src/com/fsck/k9/Preferences.java index 99d066d08..1a2b0fe13 100644 --- a/src/com/fsck/k9/Preferences.java +++ b/src/com/fsck/k9/Preferences.java @@ -15,6 +15,7 @@ import android.util.Log; import com.fsck.k9.mail.Store; import com.fsck.k9.preferences.Editor; import com.fsck.k9.preferences.Storage; +import com.fsck.k9.security.LocalKeyStore; public class Preferences { @@ -128,6 +129,7 @@ public class Preferences { Store.removeAccount(account); account.delete(this); + LocalKeyStore.getInstance().deleteCertificates(account); if (newAccount == account) { newAccount = null; diff --git a/src/com/fsck/k9/activity/setup/AccountSetupCheckSettings.java b/src/com/fsck/k9/activity/setup/AccountSetupCheckSettings.java index 9cdef8cb4..408a0784f 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupCheckSettings.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupCheckSettings.java @@ -364,13 +364,7 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList new DialogInterface.OnClickListener() { public void onClick(DialogInterface dialog, int which) { try { - Uri uri = null; - if (mDirection.equals(CheckDirection.INCOMING)) { - uri = Uri.parse(mAccount.getStoreUri()); - } else { - uri = Uri.parse(mAccount.getTransportUri()); - } - LocalKeyStore.getInstance().addCertificate(uri.getHost(), uri.getPort(), chain[0]); + LocalKeyStore.getInstance().addCertificate(mAccount, mDirection, chain[0]); } catch (CertificateException e) { showErrorDialog( R.string.account_setup_failed_dlg_certificate_message_fmt, diff --git a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java index 4ad0a2cf9..9a01140e2 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java @@ -26,6 +26,7 @@ import com.fsck.k9.mail.store.Pop3Store; import com.fsck.k9.mail.store.WebDavStore; import com.fsck.k9.mail.store.ImapStore.ImapStoreSettings; import com.fsck.k9.mail.store.WebDavStore.WebDavStoreSettings; +import com.fsck.k9.security.LocalKeyStore; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -428,6 +429,7 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener mWebdavMailboxPathView.getText().toString()); } + LocalKeyStore.getInstance().deleteCertificate(mAccount, host, port, CheckDirection.INCOMING); ServerSettings settings = new ServerSettings(mStoreType, host, port, connectionSecurity, authType, username, password, extra); diff --git a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java index 460c5bdb4..5937ece03 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java @@ -18,6 +18,7 @@ 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; +import com.fsck.k9.security.LocalKeyStore; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -309,8 +310,10 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, if (mRequireLoginView.isChecked()) { userInfo = usernameEnc + ":" + passwordEnc + ":" + authType; } - uri = new URI(smtpSchemes[securityType], userInfo, mServerView.getText().toString(), - Integer.parseInt(mPortView.getText().toString()), null, null, null); + String newHost = mServerView.getText().toString(); + int newPort = Integer.parseInt(mPortView.getText().toString()); + uri = new URI(smtpSchemes[securityType], userInfo, newHost, newPort, null, null, null); + LocalKeyStore.getInstance().deleteCertificate(mAccount, newHost, newPort, CheckDirection.OUTGOING); mAccount.setTransportUri(uri.toString()); AccountSetupCheckSettings.actionCheckSettings(this, mAccount, CheckDirection.OUTGOING); } catch (UnsupportedEncodingException enc) { diff --git a/src/com/fsck/k9/security/LocalKeyStore.java b/src/com/fsck/k9/security/LocalKeyStore.java index 2f3282cbc..ac6e755f0 100644 --- a/src/com/fsck/k9/security/LocalKeyStore.java +++ b/src/com/fsck/k9/security/LocalKeyStore.java @@ -14,9 +14,12 @@ import java.security.cert.X509Certificate; import org.apache.commons.io.IOUtils; import android.content.Context; +import android.net.Uri; import android.util.Log; +import com.fsck.k9.Account; import com.fsck.k9.K9; +import com.fsck.k9.activity.setup.AccountSetupCheckSettings.CheckDirection; public class LocalKeyStore { private static final LocalKeyStore sInstance = new LocalKeyStore(); @@ -81,9 +84,18 @@ public class LocalKeyStore { throw new CertificateException( "Certificate not added because key store not initialized"); } - java.io.OutputStream keyStoreStream = null; try { mKeyStore.setCertificateEntry(getCertKey(host, port), certificate); + } catch (KeyStoreException e) { + throw new CertificateException( + "Failed to add certificate to local key store", e); + } + writeCertificateFile(); + } + + private void writeCertificateFile() throws CertificateException { + java.io.OutputStream keyStoreStream = null; + try { keyStoreStream = new java.io.FileOutputStream(mKeyStoreFile); mKeyStore.store(keyStoreStream, "".toCharArray()); } catch (FileNotFoundException e) { @@ -106,8 +118,19 @@ public class LocalKeyStore { } } - public synchronized boolean isValidCertificate(Certificate certificate, String host, - int port) { + public void addCertificate(Account account, CheckDirection direction, + X509Certificate certificate) throws CertificateException { + Uri uri = null; + if (direction.equals(CheckDirection.INCOMING)) { + uri = Uri.parse(account.getStoreUri()); + } else { + uri = Uri.parse(account.getTransportUri()); + } + addCertificate(uri.getHost(), uri.getPort(), certificate); + } + + public synchronized boolean isValidCertificate(Certificate certificate, + String host, int port) { if (mKeyStore == null) { return false; } @@ -123,4 +146,52 @@ public class LocalKeyStore { private static String getCertKey(String host, int port) { return host + ":" + port; } + + public synchronized void deleteCertificate(String oldHost, int oldPort) { + if (mKeyStore == null) { + return; + } + try { + mKeyStore.deleteEntry(getCertKey(oldHost, oldPort)); + writeCertificateFile(); + } catch (KeyStoreException e) { + // Ignore: most likely there was no cert. found + } catch (CertificateException e) { + Log.e(K9.LOG_TAG, "Error updating the local key store file", e); + } + } + + /** + * Examine the existing settings for an account. If the old host/port is different from + * the new host/port, then try and delete any (possibly non-existent) certificate stored + * for the old host/port. + * @param account + * @param newHost + * @param newPort + * @param direction + */ + public void deleteCertificate(Account account, String newHost, int newPort, CheckDirection direction) { + Uri uri = null; + if (direction.equals(CheckDirection.INCOMING)) { + uri = Uri.parse(account.getStoreUri()); + } else { + uri = Uri.parse(account.getTransportUri()); + } + String oldHost = uri.getHost(); + int oldPort = uri.getPort(); + if (!newHost.equals(oldHost) || newPort != oldPort) { + deleteCertificate(oldHost, oldPort); + } + } + + /** + * Examine the settings for the account and attempt to delete (possibly non-existent) + * certificates for the incoming and outgoing servers. + * @param account + */ + public void deleteCertificates(Account account) { + Uri uri = Uri.parse(account.getStoreUri()); + deleteCertificate(uri.getHost(), uri.getPort()); + uri = Uri.parse(account.getTransportUri()); + deleteCertificate(uri.getHost(), uri.getPort()); } }