Merge pull request thunderbird#3772 from k9mail/cleanup-localkeystore

Clean up LocalKeyStore and related
ippschi · Dec 2, 2018 · e6e0d7c · e6e0d7c
2 parents aa8621c + 8fd2b56
commit e6e0d7c
Show file tree

Hide file tree

Showing 15 changed files with 164 additions and 179 deletions.
diff --git a/app/core/src/main/java/com/fsck/k9/Core.kt b/app/core/src/main/java/com/fsck/k9/Core.kt
@@ -65,7 +65,6 @@ object Core : KoinComponent {
 
     fun init(context: Context) {
         BinaryTempFileBody.setTempDirectory(context.cacheDir)
-        LocalKeyStore.setKeyStoreLocation(context.getDir("KeyStore", Context.MODE_PRIVATE).toString())
 
         setServicesEnabled(context)
         registerReceivers(context)

diff --git a/app/core/src/main/java/com/fsck/k9/KoinModule.kt b/app/core/src/main/java/com/fsck/k9/KoinModule.kt
@@ -3,6 +3,10 @@ package com.fsck.k9
 import android.content.Context
 import com.fsck.k9.helper.Contacts
 import com.fsck.k9.mail.power.PowerManager
+import com.fsck.k9.mail.ssl.DefaultTrustedSocketFactory
+import com.fsck.k9.mail.ssl.LocalKeyStore
+import com.fsck.k9.mail.ssl.TrustManagerFactory
+import com.fsck.k9.mail.ssl.TrustedSocketFactory
 import com.fsck.k9.mailstore.LocalStoreProvider
 import com.fsck.k9.mailstore.StorageManager
 import com.fsck.k9.power.TracingPowerManager
@@ -15,4 +19,8 @@ val mainModule = applicationContext {
     bean { LocalStoreProvider() }
     bean { TracingPowerManager.getPowerManager(get()) as PowerManager }
     bean { Contacts.getInstance(get()) }
+    bean { LocalKeyStore.createInstance(get()) }
+    bean { TrustManagerFactory.createInstance(get()) }
+    bean { LocalKeyStoreManager(get()) }
+    bean { DefaultTrustedSocketFactory(get(), get()) as TrustedSocketFactory }
 }
diff --git a/app/core/src/main/java/com/fsck/k9/LocalKeyStoreManager.kt b/app/core/src/main/java/com/fsck/k9/LocalKeyStoreManager.kt
@@ -7,18 +7,17 @@ import java.security.cert.CertificateException
 import java.security.cert.X509Certificate
 
 class LocalKeyStoreManager(
-        val localKeyStore: LocalKeyStore
+        private val localKeyStore: LocalKeyStore
 ) {
     /**
      * Add a new certificate for the incoming or outgoing server to the local key store.
      */
     @Throws(CertificateException::class)
     fun addCertificate(account: Account, direction: MailServerDirection, certificate: X509Certificate) {
-        val uri: Uri
-        if (direction === MailServerDirection.INCOMING) {
-            uri = Uri.parse(account.storeUri)
+        val uri = if (direction === MailServerDirection.INCOMING) {
+            Uri.parse(account.storeUri)
         } else {
-            uri = Uri.parse(account.transportUri)
+            Uri.parse(account.transportUri)
         }
         localKeyStore.addCertificate(uri.host, uri.port, certificate)
     }
@@ -29,11 +28,10 @@ class LocalKeyStoreManager(
      * old host/port.
      */
     fun deleteCertificate(account: Account, newHost: String, newPort: Int, direction: MailServerDirection) {
-        val uri: Uri
-        if (direction === MailServerDirection.INCOMING) {
-            uri = Uri.parse(account.storeUri)
+        val uri = if (direction === MailServerDirection.INCOMING) {
+            Uri.parse(account.storeUri)
         } else {
-            uri = Uri.parse(account.transportUri)
+            Uri.parse(account.transportUri)
         }
         val oldHost = uri.host
         val oldPort = uri.port

diff --git a/app/core/src/main/java/com/fsck/k9/notification/CoreKoinModule.kt b/app/core/src/main/java/com/fsck/k9/notification/CoreKoinModule.kt
@@ -22,8 +22,6 @@ val coreNotificationModule = applicationContext {
         )
     }
     bean { AccountPreferenceSerializer(get(), get()) }
-    bean { LocalKeyStore.getInstance() }
-    bean { LocalKeyStoreManager(get()) }
     bean { CertificateErrorNotifications(get(), get(), get()) }
     bean { AuthenticationErrorNotifications(get(), get(), get()) }
     bean { SyncNotifications(get(), get(), get()) }

diff --git a/app/k9mail/src/main/java/com/fsck/k9/backends/ImapBackendFactory.kt b/app/k9mail/src/main/java/com/fsck/k9/backends/ImapBackendFactory.kt
@@ -12,6 +12,7 @@ import com.fsck.k9.mail.ServerSettings
 import com.fsck.k9.mail.oauth.OAuth2TokenProvider
 import com.fsck.k9.mail.power.PowerManager
 import com.fsck.k9.mail.ssl.DefaultTrustedSocketFactory
+import com.fsck.k9.mail.ssl.TrustedSocketFactory
 import com.fsck.k9.mail.store.imap.ImapStore
 import com.fsck.k9.mail.transport.smtp.SmtpTransport
 import com.fsck.k9.mail.transport.smtp.SmtpTransportUriCreator
@@ -21,7 +22,8 @@ import com.fsck.k9.mailstore.K9BackendStorageFactory
 class ImapBackendFactory(
         private val context: Context,
         private val powerManager: PowerManager,
-        private val backendStorageFactory: K9BackendStorageFactory
+        private val backendStorageFactory: K9BackendStorageFactory,
+        private val trustedSocketFactory: TrustedSocketFactory
 ) : BackendFactory {
     override val transportUriPrefix = "smtp"
 
@@ -39,7 +41,7 @@ class ImapBackendFactory(
         return ImapStore(
                 serverSettings,
                 account,
-                DefaultTrustedSocketFactory(context),
+                trustedSocketFactory,
                 context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager,
                 oAuth2TokenProvider
         )
@@ -48,7 +50,7 @@ class ImapBackendFactory(
     private fun createSmtpTransport(account: Account): SmtpTransport {
         val serverSettings = decodeTransportUri(account.transportUri)
         val oauth2TokenProvider: OAuth2TokenProvider? = null
-        return SmtpTransport(serverSettings, account, DefaultTrustedSocketFactory(context), oauth2TokenProvider)
+        return SmtpTransport(serverSettings, account, trustedSocketFactory, oauth2TokenProvider)
     }
 
     override fun decodeStoreUri(storeUri: String): ServerSettings {

diff --git a/app/k9mail/src/main/java/com/fsck/k9/backends/KoinModule.kt b/app/k9mail/src/main/java/com/fsck/k9/backends/KoinModule.kt
@@ -12,7 +12,7 @@ val backendsModule = applicationContext {
                         "webdav" to get<WebDavBackendFactory>()
                 ))
     }
-    bean { ImapBackendFactory(get(), get(), get()) }
+    bean { ImapBackendFactory(get(), get(), get(), get()) }
     bean { Pop3BackendFactory(get(), get()) }
-    bean { WebDavBackendFactory(get()) }
+    bean { WebDavBackendFactory(get(), get()) }
 }
diff --git a/app/k9mail/src/main/java/com/fsck/k9/backends/Pop3BackendFactory.kt b/app/k9mail/src/main/java/com/fsck/k9/backends/Pop3BackendFactory.kt
@@ -1,6 +1,5 @@
 package com.fsck.k9.backends
 
-import android.content.Context
 import com.fsck.k9.Account
 import com.fsck.k9.backend.BackendFactory
 import com.fsck.k9.backend.api.Backend
@@ -9,16 +8,16 @@ import com.fsck.k9.backend.pop3.Pop3StoreUriCreator
 import com.fsck.k9.backend.pop3.Pop3StoreUriDecoder
 import com.fsck.k9.mail.ServerSettings
 import com.fsck.k9.mail.oauth.OAuth2TokenProvider
-import com.fsck.k9.mail.ssl.DefaultTrustedSocketFactory
+import com.fsck.k9.mail.ssl.TrustedSocketFactory
 import com.fsck.k9.mail.store.pop3.Pop3Store
 import com.fsck.k9.mail.transport.smtp.SmtpTransport
 import com.fsck.k9.mail.transport.smtp.SmtpTransportUriCreator
 import com.fsck.k9.mail.transport.smtp.SmtpTransportUriDecoder
 import com.fsck.k9.mailstore.K9BackendStorageFactory
 
 class Pop3BackendFactory(
-        private val context: Context,
-        private val backendStorageFactory: K9BackendStorageFactory
+        private val backendStorageFactory: K9BackendStorageFactory,
+        private val trustedSocketFactory: TrustedSocketFactory
 ) : BackendFactory {
     override val transportUriPrefix = "smtp"
 
@@ -32,13 +31,13 @@ class Pop3BackendFactory(
 
     private fun createPop3Store(account: Account): Pop3Store {
         val serverSettings = decodeStoreUri(account.storeUri)
-        return Pop3Store(serverSettings, account, DefaultTrustedSocketFactory(context))
+        return Pop3Store(serverSettings, account, trustedSocketFactory)
     }
 
     private fun createSmtpTransport(account: Account): SmtpTransport {
         val serverSettings = decodeTransportUri(account.transportUri)
         val oauth2TokenProvider: OAuth2TokenProvider? = null
-        return SmtpTransport(serverSettings, account, DefaultTrustedSocketFactory(context), oauth2TokenProvider)
+        return SmtpTransport(serverSettings, account, trustedSocketFactory, oauth2TokenProvider)
     }
 
     override fun decodeStoreUri(storeUri: String): ServerSettings {

diff --git a/app/k9mail/src/main/java/com/fsck/k9/backends/WebDavBackendFactory.kt b/app/k9mail/src/main/java/com/fsck/k9/backends/WebDavBackendFactory.kt
@@ -7,25 +7,29 @@ import com.fsck.k9.backend.webdav.WebDavBackend
 import com.fsck.k9.backend.webdav.WebDavStoreUriCreator
 import com.fsck.k9.backend.webdav.WebDavStoreUriDecoder
 import com.fsck.k9.mail.ServerSettings
+import com.fsck.k9.mail.ssl.TrustManagerFactory
 import com.fsck.k9.mail.store.webdav.WebDavStore
 import com.fsck.k9.mail.store.webdav.WebDavStoreSettings
 import com.fsck.k9.mail.transport.WebDavTransport
 import com.fsck.k9.mailstore.K9BackendStorageFactory
 
-class WebDavBackendFactory(private val backendStorageFactory: K9BackendStorageFactory) : BackendFactory {
+class WebDavBackendFactory(
+        private val backendStorageFactory: K9BackendStorageFactory,
+        private val trustManagerFactory: TrustManagerFactory
+) : BackendFactory {
     override val transportUriPrefix = "webdav"
 
     override fun createBackend(account: Account): Backend {
         val accountName = account.displayName
         val backendStorage = backendStorageFactory.createBackendStorage(account)
         val serverSettings = WebDavStoreUriDecoder.decode(account.storeUri)
         val webDavStore = createWebDavStore(serverSettings, account)
-        val webDavTransport = WebDavTransport(serverSettings, account)
+        val webDavTransport = WebDavTransport(trustManagerFactory, serverSettings, account)
         return WebDavBackend(accountName, backendStorage, webDavStore, webDavTransport)
     }
 
     private fun createWebDavStore(serverSettings: WebDavStoreSettings, account: Account): WebDavStore {
-        return WebDavStore(serverSettings, account)
+        return WebDavStore(trustManagerFactory, serverSettings, account)
     }
 
     override fun decodeStoreUri(storeUri: String): ServerSettings {

diff --git a/mail/common/src/main/java/com/fsck/k9/mail/ssl/DefaultTrustedSocketFactory.java b/mail/common/src/main/java/com/fsck/k9/mail/ssl/DefaultTrustedSocketFactory.java
@@ -31,10 +31,10 @@
  * On more modern versions of Android we keep the system configuration.
  */
 public class DefaultTrustedSocketFactory implements TrustedSocketFactory {
-    protected static final String[] ENABLED_CIPHERS;
-    protected static final String[] ENABLED_PROTOCOLS;
+    private static final String[] ENABLED_CIPHERS;
+    private static final String[] ENABLED_PROTOCOLS;
 
-    protected static final String[] ORDERED_KNOWN_CIPHERS = {
+    private static final String[] ORDERED_KNOWN_CIPHERS = {
             "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
             "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
             "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
@@ -61,7 +61,7 @@ public class DefaultTrustedSocketFactory implements TrustedSocketFactory {
             "TLS_RSA_WITH_AES_128_CBC_SHA",
     };
 
-    protected static final String[] BLACKLISTED_CIPHERS = {
+    private static final String[] BLACKLISTED_CIPHERS = {
             "SSL_RSA_WITH_DES_CBC_SHA",
             "SSL_DHE_RSA_WITH_DES_CBC_SHA",
             "SSL_DHE_DSS_WITH_DES_CBC_SHA",
@@ -85,11 +85,11 @@ public class DefaultTrustedSocketFactory implements TrustedSocketFactory {
             "TLS_RSA_WITH_NULL_SHA256"
     };
 
-    protected static final String[] ORDERED_KNOWN_PROTOCOLS = {
+    private static final String[] ORDERED_KNOWN_PROTOCOLS = {
             "TLSv1.2", "TLSv1.1", "TLSv1"
     };
 
-    protected static final String[] BLACKLISTED_PROTOCOLS = {
+    private static final String[] BLACKLISTED_PROTOCOLS = {
             "SSLv3"
     };
 
@@ -128,15 +128,19 @@ public class DefaultTrustedSocketFactory implements TrustedSocketFactory {
 
     }
 
-    public DefaultTrustedSocketFactory(Context context) {
+    private final Context context;
+    private final TrustManagerFactory trustManagerFactory;
+
+    public DefaultTrustedSocketFactory(Context context, TrustManagerFactory trustManagerFactory) {
         this.context = context;
+        this.trustManagerFactory = trustManagerFactory;
     }
 
     private static boolean hasWeakSslImplementation() {
         return android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP;
     }
 
-    protected static String[] reorder(String[] enabled, String[] known, String[] blacklisted) {
+    private static String[] reorder(String[] enabled, String[] known, String[] blacklisted) {
         List<String> unknown = new ArrayList<>();
         Collections.addAll(unknown, enabled);
 
@@ -159,7 +163,7 @@ protected static String[] reorder(String[] enabled, String[] known, String[] bla
         // start showing up in the future.
         result.addAll(unknown);
 
-        return result.toArray(new String[result.size()]);
+        return result.toArray(new String[0]);
     }
 
     protected static String[] remove(String[] enabled, String[] blacklisted) {
@@ -173,15 +177,13 @@ protected static String[] remove(String[] enabled, String[] blacklisted) {
             }
         }
 
-        return items.toArray(new String[items.size()]);
+        return items.toArray(new String[0]);
     }
 
-    private Context context;
-
     public Socket createSocket(Socket socket, String host, int port, String clientCertificateAlias)
             throws NoSuchAlgorithmException, KeyManagementException, MessagingException, IOException {
 
-        TrustManager[] trustManagers = new TrustManager[] { TrustManagerFactory.get(host, port) };
+        TrustManager[] trustManagers = new TrustManager[] { trustManagerFactory.getTrustManagerForDomain(host, port) };
         KeyManager[] keyManagers = null;
         if (!TextUtils.isEmpty(clientCertificateAlias)) {
             keyManagers = new KeyManager[] { new KeyChainKeyManager(context, clientCertificateAlias) };