Update comments in HttpClientBuilder.kt for clarity

- Add context cache using Guava CacheBuilder
- Cache SSLContext in getContext method

app/src/main/kotlin/at/bitfire/davdroid/di/CustomCertManagerModule.kt

@@ -0,0 +1,65 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.di
+
+import android.content.Context
+import at.bitfire.cert4android.CustomCertManager
+import at.bitfire.cert4android.CustomCertStore
+import at.bitfire.cert4android.SettingsProvider
+import at.bitfire.davdroid.BuildConfig
+import at.bitfire.davdroid.settings.Settings
+import at.bitfire.davdroid.settings.SettingsManager
+import at.bitfire.davdroid.ui.ForegroundTracker
+import dagger.Module
+import dagger.Provides
+import dagger.hilt.InstallIn
+import dagger.hilt.android.qualifiers.ApplicationContext
+import dagger.hilt.components.SingletonComponent
+import okhttp3.internal.tls.OkHostnameVerifier
+import java.util.Optional
+import javax.inject.Singleton
+
+@Module
+@InstallIn(SingletonComponent::class)
+/**
+ * cert4android integration module
+ */
+class CustomCertManagerModule {
+
+    @Provides
+    @Singleton
+    fun customCertManager(
+        @ApplicationContext context: Context,
+        settings: SettingsManager
+    ): Optional<CustomCertManager> =
+        if (BuildConfig.allowCustomCerts)
+            Optional.of(
+                CustomCertManager(
+                certStore = CustomCertStore.getInstance(context),
+                settings = object : SettingsProvider {
+
+                    override val appInForeground: Boolean
+                        get() = ForegroundTracker.inForeground.value
+
+                    override val trustSystemCerts: Boolean
+                        get() = !settings.getBoolean(Settings.DISTRUST_SYSTEM_CERTIFICATES)
+
+                }
+            ))
+        else
+            Optional.empty()
+
+    @Provides
+    @Singleton
+    fun customHostnameVerifier(
+        customCertManager: Optional<CustomCertManager>
+    ): Optional<CustomCertManager.HostnameVerifier> =
+        if (BuildConfig.allowCustomCerts && customCertManager.isPresent) {
+            val hostnameVerifier = customCertManager.get().HostnameVerifier(OkHostnameVerifier)
+            Optional.of(hostnameVerifier)
+        } else
+            Optional.empty()
+
+}

app/src/main/kotlin/at/bitfire/davdroid/network/ConnectionSecurityContext.kt

@@ -0,0 +1,16 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.network
+
+import javax.net.ssl.HostnameVerifier
+import javax.net.ssl.SSLSocketFactory
+import javax.net.ssl.X509TrustManager
+
+class ConnectionSecurityContext(
+    val sslSocketFactory: SSLSocketFactory?,
+    val trustManager: X509TrustManager?,
+    val hostnameVerifier: HostnameVerifier?,
+    val disableHttp2: Boolean
+)

app/src/main/kotlin/at/bitfire/davdroid/network/ConnectionSecurityManager.kt

@@ -0,0 +1,75 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.network
+
+import at.bitfire.cert4android.CustomCertManager
+import com.google.common.cache.Cache
+import com.google.common.cache.CacheBuilder
+import java.security.KeyStore
+import java.util.Optional
+import java.util.logging.Level
+import java.util.logging.Logger
+import javax.inject.Inject
+import javax.inject.Singleton
+import javax.net.ssl.KeyManager
+import javax.net.ssl.SSLContext
+import javax.net.ssl.TrustManagerFactory
+import javax.net.ssl.X509TrustManager
+import kotlin.jvm.optionals.getOrNull
+
+/**
+ * Caching provider for [SSLContext].
+ */
+@Singleton
+class ConnectionSecurityManager @Inject constructor(
+    private val customHostnameVerifier: Optional<CustomCertManager.HostnameVerifier>,
+    customTrustManager: Optional<CustomCertManager>,
+    private val keyManagerFactory: ClientCertKeyManager.Factory,
+    private val logger: Logger,
+) {
+
+    private val contextCache: Cache<Optional<String>, ConnectionSecurityContext> = CacheBuilder.newBuilder()
+        .build()
+    private val trustManager = customTrustManager.getOrNull() ?: defaultTrustManager()
+
+    fun getContext(certificateAlias: String?) =
+        // cache SSLContext by certificate alias
+        contextCache.get(Optional.ofNullable(certificateAlias)) {
+            val clientKeyManager = certificateAlias?.let { getClientKeyManager(it) }
+
+            val sslContext = SSLContext.getInstance("TLS").apply {
+                init(
+                    /* km = */ if (clientKeyManager != null) arrayOf(clientKeyManager) else null,
+                    /* tm = */ arrayOf(trustManager),
+                    /* random = */ null
+                )
+            }
+
+            ConnectionSecurityContext(
+                sslSocketFactory = sslContext.socketFactory,
+                trustManager = trustManager,
+                hostnameVerifier = customHostnameVerifier.getOrNull(),
+                disableHttp2 = certificateAlias != null
+            )
+        }
+
+    fun getClientKeyManager(alias: String): KeyManager? =
+        try {
+            val manager = keyManagerFactory.create(alias)
+            logger.fine("Using certificate $alias for authentication")
+
+            manager
+        } catch (e: IllegalArgumentException) {
+            logger.log(Level.SEVERE, "Couldn't create KeyManager for certificate $alias", e)
+            null
+        }
+
+    private fun defaultTrustManager(): X509TrustManager {
+        val factory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm())
+        factory.init(null as KeyStore?)
+        return factory.trustManagers.filterIsInstance<X509TrustManager>().first()
+    }
+
+}

app/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt

@@ -5,22 +5,16 @@
 package at.bitfire.davdroid.network
 
 import android.accounts.Account
-import android.content.Context
 import androidx.annotation.WorkerThread
-import at.bitfire.cert4android.CustomCertManager
-import at.bitfire.cert4android.CustomCertStore
 import at.bitfire.dav4jvm.okhttp.BasicDigestAuthHandler
 import at.bitfire.dav4jvm.okhttp.UrlUtils
-import at.bitfire.davdroid.BuildConfig
 import at.bitfire.davdroid.di.IoDispatcher
 import at.bitfire.davdroid.settings.AccountSettings
 import at.bitfire.davdroid.settings.Credentials
 import at.bitfire.davdroid.settings.Settings
 import at.bitfire.davdroid.settings.SettingsManager
-import at.bitfire.davdroid.ui.ForegroundTracker
 import com.google.common.net.HttpHeaders
 import com.google.errorprone.annotations.MustBeClosed
-import dagger.hilt.android.qualifiers.ApplicationContext
 import io.ktor.client.HttpClient
 import io.ktor.client.engine.okhttp.OkHttp
 import io.ktor.client.plugins.contentnegotiation.ContentNegotiation
@@ -35,20 +29,13 @@ import okhttp3.Interceptor
 import okhttp3.OkHttpClient
 import okhttp3.Protocol
 import okhttp3.brotli.BrotliInterceptor
-import okhttp3.internal.tls.OkHostnameVerifier
 import okhttp3.logging.HttpLoggingInterceptor
 import java.net.InetSocketAddress
 import java.net.Proxy
-import java.security.KeyStore
 import java.util.concurrent.TimeUnit
 import java.util.logging.Level
 import java.util.logging.Logger
 import javax.inject.Inject
-import javax.net.ssl.HostnameVerifier
-import javax.net.ssl.KeyManager
-import javax.net.ssl.SSLContext
-import javax.net.ssl.TrustManagerFactory
-import javax.net.ssl.X509TrustManager
 
 /**
  * Builder for the HTTP client.
@@ -60,10 +47,9 @@ import javax.net.ssl.X509TrustManager
  */
 class HttpClientBuilder @Inject constructor(
     private val accountSettingsFactory: AccountSettings.Factory,
-    @ApplicationContext private val context: Context,
+    private val connectionSecurityManager: ConnectionSecurityManager,
     defaultLogger: Logger,
     @IoDispatcher private val ioDispatcher: CoroutineDispatcher,
-    private val keyManagerFactory: ClientCertKeyManager.Factory,
     private val oAuthInterceptorFactory: OAuthInterceptor.Factory,
     private val settingsManager: SettingsManager
 ) {
@@ -283,72 +269,29 @@ class HttpClientBuilder @Inject constructor(
     }
 
     private fun buildConnectionSecurity(okBuilder: OkHttpClient.Builder) {
-        // allow cleartext and TLS 1.2+
+        // Allow cleartext and TLS 1.2+
         okBuilder.connectionSpecs(listOf(
             ConnectionSpec.CLEARTEXT,
             ConnectionSpec.MODERN_TLS
         ))
 
-        // client certificate
-        val clientKeyManager: KeyManager? = certificateAlias?.let { alias ->
-            try {
-                val manager = keyManagerFactory.create(alias)
-                logger.fine("Using certificate $alias for authentication")
+        /* Set SSLSocketFactory, TrustManager and HostnameVerifier (if needed).
+         * We shouldn't create these things here, because
+         *
+         * a. it involves complex logic that should be the responsibility of a dedicated class, and
+         * b. we need to cache the instances because otherwise, HTTPS connection are not used
+         *    correctly. okhttp checks the SSLSocketFactory/TrustManager of a connection in the pool
+         *    and creates a new connection when they have changed. */
+        val securityContext = connectionSecurityManager.getContext(certificateAlias)
 
-                // HTTP/2 doesn't support client certificates (yet)
-                // see https://datatracker.ietf.org/doc/draft-ietf-httpbis-secondary-server-certs/
-                okBuilder.protocols(listOf(Protocol.HTTP_1_1))
+        if (securityContext.disableHttp2)
+            okBuilder.protocols(listOf(Protocol.HTTP_1_1))
 
-                manager
-            } catch (e: IllegalArgumentException) {
-                logger.log(Level.SEVERE, "Couldn't create KeyManager for certificate $alias", e)
-                null
-            }
-        }
+        if (securityContext.sslSocketFactory != null && securityContext.trustManager != null)
+            okBuilder.sslSocketFactory(securityContext.sslSocketFactory, securityContext.trustManager)
 
-        // select trust manager and hostname verifier depending on whether custom certificates are allowed
-        val customTrustManager: X509TrustManager?
-        val customHostnameVerifier: HostnameVerifier?
-
-        if (BuildConfig.allowCustomCerts) {
-            // use cert4android for custom certificate handling
-            customTrustManager = CustomCertManager(
-                certStore = CustomCertStore.getInstance(context),
-                trustSystemCerts = !settingsManager.getBoolean(Settings.DISTRUST_SYSTEM_CERTIFICATES),
-                appInForeground = ForegroundTracker.inForeground
-            )
-            // allow users to accept certificates with wrong host names
-            customHostnameVerifier = customTrustManager.HostnameVerifier(OkHostnameVerifier)
-
-        } else {
-            // no custom certificates, use default trust manager and hostname verifier
-            customTrustManager = null
-            customHostnameVerifier = null
-        }
-
-        // change settings only if we have at least only one custom component
-        if (clientKeyManager != null || customTrustManager != null) {
-            val trustManager = customTrustManager ?: defaultTrustManager()
-
-            // use trust manager and client key manager (if defined) for TLS connections
-            val sslContext = SSLContext.getInstance("TLS")
-            sslContext.init(
-                /* km = */ if (clientKeyManager != null) arrayOf(clientKeyManager) else null,
-                /* tm = */ arrayOf(trustManager),
-                /* random = */ null
-            )
-            okBuilder.sslSocketFactory(sslContext.socketFactory, trustManager)
-        }
-
-        // also add the custom hostname verifier (if defined)
-        if (customHostnameVerifier != null)
-            okBuilder.hostnameVerifier(customHostnameVerifier)
-    }
-
-    private fun defaultTrustManager(): X509TrustManager {
-        val factory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm())
-        factory.init(null as KeyStore?)
-        return factory.trustManagers.filterIsInstance<X509TrustManager>().first()
+        if (securityContext.hostnameVerifier != null)
+            okBuilder.hostnameVerifier(securityContext.hostnameVerifier)
     }
 
     private fun buildProxy(okBuilder: OkHttpClient.Builder) {

gradle/libs.versions.toml

@@ -18,7 +18,7 @@ androidx-test-runner = "1.7.0"
 androidx-test-rules = "1.7.0"
 androidx-test-junit = "1.3.0"
 androidx-work = "2.11.0"
-bitfire-cert4android = "42d883e958"
+bitfire-cert4android = "75cc6913fd"
 bitfire-dav4jvm = "acf8e4ef9b"
 bitfire-synctools = "25b92ef99a"
 compose-accompanist = "0.37.3"