diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 90bad164a..32232ee16 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -27,7 +27,8 @@ android { minSdk = 24 // Android 7.0 targetSdk = 36 // Android 16 - buildConfigField("boolean", "customCertsUI", "true") + // whether the build supports and allows to use custom certificates + buildConfigField("boolean", "allowCustomCerts", "true") testInstrumentationRunner = "at.bitfire.davdroid.HiltTestRunner" } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/network/ClientCertKeyManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/network/ClientCertKeyManager.kt index 0859849f6..5b4c3205c 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/network/ClientCertKeyManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/network/ClientCertKeyManager.kt @@ -6,22 +6,31 @@ package at.bitfire.davdroid.network import android.content.Context import android.security.KeyChain +import android.security.KeyChainException import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import dagger.hilt.android.qualifiers.ApplicationContext import java.net.Socket import java.security.Principal +import java.security.PrivateKey +import java.security.cert.X509Certificate +import java.util.logging.Level +import java.util.logging.Logger import javax.net.ssl.X509ExtendedKeyManager /** - * KeyManager that provides a client certificate and private key from the Android KeyChain. + * KeyManager that provides a client certificate and private key from the Android [KeyChain]. * - * @throws IllegalArgumentException if the alias doesn't exist or is not accessible + * Requests for certificates / private keys for other aliases than the specified one + * will be ignored. + * + * @param alias alias of the desired certificate / private key */ class ClientCertKeyManager @AssistedInject constructor( @Assisted private val alias: String, - @ApplicationContext private val context: Context + @ApplicationContext private val context: Context, + private val logger: Logger ): X509ExtendedKeyManager() { @AssistedFactory @@ -29,19 +38,42 @@ class ClientCertKeyManager @AssistedInject constructor( fun create(alias: String): ClientCertKeyManager } - val certs = KeyChain.getCertificateChain(context, alias) ?: throw IllegalArgumentException("Alias doesn't exist or not accessible: $alias") - val key = KeyChain.getPrivateKey(context, alias) ?: throw IllegalArgumentException("Alias doesn't exist or not accessible: $alias") - override fun getServerAliases(p0: String?, p1: Array?): Array? = null override fun chooseServerAlias(p0: String?, p1: Array?, p2: Socket?) = null override fun getClientAliases(p0: String?, p1: Array?) = arrayOf(alias) override fun chooseClientAlias(p0: Array?, p1: Array?, p2: Socket?) = alias - override fun getCertificateChain(forAlias: String?) = - certs.takeIf { forAlias == alias } + override fun getCertificateChain(forAlias: String): Array? { + if (forAlias != alias) + return null - override fun getPrivateKey(forAlias: String?) = - key.takeIf { forAlias == alias } + return try { + KeyChain.getCertificateChain(context, alias).also { result -> + if (result == null) + logger.warning("Couldn't obtain certificate chain for alias $alias") + } + } catch (e: KeyChainException) { + // Android + if (result == null) + logger.log(Level.WARNING, "Couldn't obtain private key for alias $alias") + } + } catch (e: KeyChainException) { + // Android Credentials, updateAuthState: ((AuthState) -> Unit)? = null): HttpClientBuilder { val credentials = getCredentials() if (credentials.authState != null) { @@ -130,6 +137,7 @@ class HttpClientBuilder @Inject constructor( } private var followRedirects = false + fun followRedirects(follow: Boolean): HttpClientBuilder { followRedirects = follow return this @@ -224,7 +232,8 @@ class HttpClientBuilder @Inject constructor( // app-wide custom proxy support buildProxy(okBuilder) - // add authentication + // add connection security (including client certificates) and authentication + buildConnectionSecurity(okBuilder) buildAuthentication(okBuilder) // add network logging, if requested @@ -246,15 +255,17 @@ class HttpClientBuilder @Inject constructor( // basic/digest auth and OAuth authenticationInterceptor?.let { okBuilder.addInterceptor(it) } authenticator?.let { okBuilder.authenticator(it) } + } + private fun buildConnectionSecurity(okBuilder: OkHttpClient.Builder) { // client certificate - val keyManager: KeyManager? = certificateAlias?.let { alias -> + val clientKeyManager: KeyManager? = certificateAlias?.let { alias -> try { val manager = keyManagerFactory.create(alias) logger.fine("Using certificate $alias for authentication") // HTTP/2 doesn't support client certificates (yet) - // see https://tools.ietf.org/html/draft-ietf-httpbis-http2-secondary-certs-04 + // see https://datatracker.ietf.org/doc/draft-ietf-httpbis-secondary-server-certs/ okBuilder.protocols(listOf(Protocol.HTTP_1_1)) manager @@ -264,25 +275,49 @@ class HttpClientBuilder @Inject constructor( } } - // cert4android integration - val certManager = CustomCertManager( - context = context, - trustSystemCerts = !settingsManager.getBoolean(Settings.DISTRUST_SYSTEM_CERTIFICATES), - appInForeground = if (BuildConfig.customCertsUI) - ForegroundTracker.inForeground // interactive mode - else - null // non-interactive mode - ) + // select trust manager and hostname verifier depending on whether custom certificates are allowed + val customTrustManager: X509TrustManager? + val customHostnameVerifier: HostnameVerifier? - val sslContext = SSLContext.getInstance("TLS") - sslContext.init( - /* km = */ if (keyManager != null) arrayOf(keyManager) else null, - /* tm = */ arrayOf(certManager), - /* random = */ null - ) - okBuilder - .sslSocketFactory(sslContext.socketFactory, certManager) - .hostnameVerifier(certManager.HostnameVerifier(OkHostnameVerifier)) + if (BuildConfig.allowCustomCerts) { + // use cert4android for custom certificate handling + customTrustManager = CustomCertManager( + context = 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().first() } private fun buildProxy(okBuilder: OkHttpClient.Builder) { diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsScreen.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsScreen.kt index 373eb16d3..15d12d681 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsScreen.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsScreen.kt @@ -67,6 +67,7 @@ import androidx.compose.ui.unit.dp import androidx.core.graphics.drawable.toBitmap import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewmodel.compose.viewModel +import at.bitfire.davdroid.BuildConfig import at.bitfire.davdroid.R import at.bitfire.davdroid.settings.Settings import at.bitfire.davdroid.ui.AppSettingsModel.PushDistributorInfo @@ -107,10 +108,11 @@ fun AppSettingsScreen( onProxyPortUpdated = model::updateProxyPort, // Security + onNavPermissionsScreen = onNavPermissionsScreen, + showCertSettings = BuildConfig.allowCustomCerts, distrustSystemCerts = model.distrustSystemCertificates().collectAsStateWithLifecycle(null).value ?: false, onDistrustSystemCertsUpdated = model::updateDistrustSystemCertificates, onResetCertificates = model::resetCertificates, - onNavPermissionsScreen = onNavPermissionsScreen, // User interface onShowNotificationSettings = onShowNotificationSettings, @@ -149,10 +151,11 @@ fun AppSettingsScreen( onProxyPortUpdated: (Int) -> Unit, // AppSettings security + onNavPermissionsScreen: () -> Unit, + showCertSettings: Boolean, distrustSystemCerts: Boolean, onDistrustSystemCertsUpdated: (Boolean) -> Unit, onResetCertificates: () -> Unit, - onNavPermissionsScreen: () -> Unit, // AppSettings UserInterface theme: Int, @@ -224,6 +227,8 @@ fun AppSettingsScreen( val resetCertificatesSuccessMessage = stringResource(R.string.app_settings_reset_certificates_success) AppSettings_Security( + onNavPermissionsScreen = onNavPermissionsScreen, + showCertSettings = showCertSettings, distrustSystemCerts = distrustSystemCerts, onDistrustSystemCertsUpdated = onDistrustSystemCertsUpdated, onResetCertificates = { @@ -231,8 +236,7 @@ fun AppSettingsScreen( coroutineScope.launch { snackbarHostState.showSnackbar(resetCertificatesSuccessMessage) } - }, - onNavPermissionsScreen = onNavPermissionsScreen + } ) val resetHintsSuccessMessage = stringResource(R.string.app_settings_reset_hints_success) @@ -282,9 +286,10 @@ fun AppSettingsScreen_Preview() { onNavUp = {}, onProxyTypeUpdated = {}, onProxyPortUpdated = {}, + onNavPermissionsScreen = {}, + showCertSettings = true, onDistrustSystemCertsUpdated = {}, onResetCertificates = {}, - onNavPermissionsScreen = {}, onThemeSelected = {}, onResetHints = {}, tasksAppName = "No tasks app", @@ -420,48 +425,51 @@ fun AppSettings_Connection( @Composable fun AppSettings_Security( + onNavPermissionsScreen: () -> Unit, + showCertSettings: Boolean, distrustSystemCerts: Boolean, onDistrustSystemCertsUpdated: (Boolean) -> Unit, - onResetCertificates: () -> Unit, - onNavPermissionsScreen: () -> Unit + onResetCertificates: () -> Unit ) { SettingsHeader(divider = true) { Text(stringResource(R.string.app_settings_security)) } - var showingDistrustWarning by remember { mutableStateOf(false) } - if (showingDistrustWarning) { - DistrustSystemCertificatesAlertDialog( - onDistrustSystemCertsRequested = { onDistrustSystemCertsUpdated(true) }, - onDismissRequested = { showingDistrustWarning = false } - ) - } - - SwitchSetting( - checked = distrustSystemCerts, - name = stringResource(R.string.app_settings_distrust_system_certs), - summaryOn = stringResource(R.string.app_settings_distrust_system_certs_on), - summaryOff = stringResource(R.string.app_settings_distrust_system_certs_off) - ) { checked -> - if (checked) { - // Show warning before enabling. - showingDistrustWarning = true - } else { - onDistrustSystemCertsUpdated(false) - } - } - - Setting( - name = stringResource(R.string.app_settings_reset_certificates), - summary = stringResource(R.string.app_settings_reset_certificates_summary), - onClick = onResetCertificates - ) - Setting( name = stringResource(R.string.app_settings_security_app_permissions), summary = stringResource(R.string.app_settings_security_app_permissions_summary), onClick = onNavPermissionsScreen ) + + if (showCertSettings) { + var showingDistrustWarning by remember { mutableStateOf(false) } + if (showingDistrustWarning) { + DistrustSystemCertificatesAlertDialog( + onDistrustSystemCertsRequested = { onDistrustSystemCertsUpdated(true) }, + onDismissRequested = { showingDistrustWarning = false } + ) + } + + SwitchSetting( + checked = distrustSystemCerts, + name = stringResource(R.string.app_settings_distrust_system_certs), + summaryOn = stringResource(R.string.app_settings_distrust_system_certs_on), + summaryOff = stringResource(R.string.app_settings_distrust_system_certs_off) + ) { checked -> + if (checked) { + // Show warning before enabling. + showingDistrustWarning = true + } else { + onDistrustSystemCertsUpdated(false) + } + } + + Setting( + name = stringResource(R.string.app_settings_reset_certificates), + summary = stringResource(R.string.app_settings_reset_certificates_summary), + onClick = onResetCertificates + ) + } } @Composable