Allow users to accept certificates that don't match the hostname (#32)

* Ask user to trust certificates if they are for a wrong hostname (but are valid) * Rename CustomHostnameVerifier.CustomHostnameVerifier to HostnameVerifier * Update dependencies, add targetSdk again (to make tests running) * Lint
bitfireAT · Oct 6, 2023 · 2bb3898 · 2bb3898
1 parent e823b78
commit 2bb3898
Show file tree

Hide file tree

Showing 9 changed files with 58 additions and 65 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -1,6 +1,6 @@
 // Top-level build file where you can add configuration options common to all sub-projects/modules.
 plugins {
-    id("com.android.library") version "8.1.1" apply false
+    id("com.android.library") version "8.1.2" apply false
     id("org.jetbrains.kotlin.android") version "1.9.0" apply false
     id("org.jetbrains.dokka") version "1.8.20" apply false
 }

diff --git a/lib/build.gradle.kts b/lib/build.gradle.kts
@@ -12,6 +12,7 @@ android {
 
     defaultConfig {
         minSdk = 21            // Android 5
+        targetSdk = 34
 
         aarMetadata {
             minCompileSdk = 29
@@ -82,14 +83,14 @@ dependencies {
     implementation("androidx.lifecycle:lifecycle-extensions:2.2.0")
     implementation("androidx.lifecycle:lifecycle-livedata-ktx:2.6.2")
     implementation("androidx.lifecycle:lifecycle-viewmodel-ktx:2.6.2")
-    implementation("com.google.android.material:material:1.9.0")
+    implementation("com.google.android.material:material:1.10.0")
     implementation("org.conscrypt:conscrypt-android:2.5.2")
 
     // Jetpack Compose
     val composeBom = platform("androidx.compose:compose-bom:2023.08.00")
     implementation(composeBom)
     androidTestImplementation(composeBom)
-    implementation("androidx.activity:activity-compose:1.7.2")
+    implementation("androidx.activity:activity-compose:1.8.0")
     implementation("androidx.compose.material:material")
     implementation("androidx.compose.runtime:runtime-livedata")
     debugImplementation("androidx.compose.ui:ui-tooling")
@@ -99,7 +100,7 @@ dependencies {
     androidTestImplementation("androidx.test:runner:1.5.2")
     androidTestImplementation("androidx.test:rules:1.5.0")
     androidTestImplementation("com.squareup.okhttp3:mockwebserver:4.11.0")
-    androidTestImplementation("io.mockk:mockk-android:1.13.7")
+    androidTestImplementation("io.mockk:mockk-android:1.13.8")
 
     testImplementation("junit:junit:4.13.2")
 }
diff --git a/lib/src/androidTest/java/at/bitfire/cert4android/CustomCertManagerTest.kt b/lib/src/androidTest/java/at/bitfire/cert4android/CustomCertManagerTest.kt
@@ -30,12 +30,12 @@ class CustomCertManagerTest {
         }
     }
 
-    val context by lazy { InstrumentationRegistry.getInstrumentation().targetContext }
+    private val context by lazy { InstrumentationRegistry.getInstrumentation().targetContext }
 
-    lateinit var certManager: CustomCertManager
-    lateinit var paranoidCertManager: CustomCertManager
+    private lateinit var certManager: CustomCertManager
+    private lateinit var paranoidCertManager: CustomCertManager
 
-    var siteCerts: List<X509Certificate>? = null
+    private var siteCerts: List<X509Certificate>? = null
     init {
         try {
             siteCerts = getSiteCertificates(URL("https://www.davx5.com"))

diff --git a/lib/src/main/java/at/bitfire/cert4android/CustomCertManager.kt b/lib/src/main/java/at/bitfire/cert4android/CustomCertManager.kt
@@ -9,6 +9,7 @@ import android.content.Context
 import kotlinx.coroutines.flow.StateFlow
 import java.security.cert.CertificateException
 import java.security.cert.X509Certificate
+import javax.net.ssl.SSLSession
 import javax.net.ssl.X509TrustManager
 
 /**
@@ -26,7 +27,7 @@ class CustomCertManager @JvmOverloads constructor(
     var appInForeground: StateFlow<Boolean>?
 ): X509TrustManager {
 
-    private val certStore = CustomCertStore.getInstance(context)
+    val certStore = CustomCertStore.getInstance(context)
 
 
     @Throws(CertificateException::class)
@@ -35,7 +36,7 @@ class CustomCertManager @JvmOverloads constructor(
     }
 
     /**
-     * Checks whether a certificate is trusted.
+     * Checks whether a certificate is trusted. Allows user to explicitly accept untrusted certificates.
      *
      * @param chain        certificate chain to check
      * @param authType     authentication type (ignored)
@@ -50,4 +51,31 @@ class CustomCertManager @JvmOverloads constructor(
 
     override fun getAcceptedIssuers() = arrayOf<X509Certificate>()
 
+
+    /**
+     * A HostnameVerifier that allows users to explicitly accept untrusted and
+     * non-matching (bad hostname) certificates.
+     */
+    inner class HostnameVerifier(
+        private val defaultHostnameVerifier: javax.net.ssl.HostnameVerifier? = null
+    ): javax.net.ssl.HostnameVerifier {
+
+        override fun verify(hostname: String, session: SSLSession): Boolean {
+            if (defaultHostnameVerifier != null && defaultHostnameVerifier.verify(hostname, session))
+                // default HostnameVerifier says trusted → OK
+                return true
+
+            Cert4Android.log.warning("Host name \"$hostname\" not verified, checking whether certificate is explicitly trusted")
+            // Allow users to explicitly accept certificates that have a bad hostname here
+            (session.peerCertificates.firstOrNull() as? X509Certificate)?.let { cert ->
+                // Check without trusting system certificates so that the user will be asked even for system-trusted certificates
+                if (certStore.isTrusted(arrayOf(cert), "RSA", false, appInForeground))
+                    return true
+            }
+
+            return false
+        }
+
+    }
+
 }
diff --git a/lib/src/main/java/at/bitfire/cert4android/CustomHostnameVerifier.kt b/lib/src/main/java/at/bitfire/cert4android/CustomHostnameVerifier.kt
diff --git a/lib/src/main/java/at/bitfire/cert4android/TrustCertificateActivity.kt b/lib/src/main/java/at/bitfire/cert4android/TrustCertificateActivity.kt
@@ -64,8 +64,7 @@ class TrustCertificateActivity : ComponentActivity() {
         }
         onBackPressedDispatcher.addCallback(this, object: OnBackPressedCallback(true) {
             override fun handleOnBackPressed() {
-                // treat "back" as reject
-                model.registerDecision(false)
+                backPressedCounter.intValue++
             }
         })
 
@@ -80,11 +79,6 @@ class TrustCertificateActivity : ComponentActivity() {
         }
     }
 
-    @Suppress("OVERRIDE_DEPRECATION")
-    override fun onBackPressed() {
-        backPressedCounter.intValue++
-    }
-
 
     @Composable
     @Preview

diff --git a/lib/src/main/java/at/bitfire/cert4android/UserDecisionRegistry.kt b/lib/src/main/java/at/bitfire/cert4android/UserDecisionRegistry.kt
@@ -92,10 +92,11 @@ class UserDecisionRegistry private constructor(
      *
      * @param cert              certificate to ask user about
      * @param launchActivity    whether to launch a [TrustCertificateActivity]
-     * @param showNotification  whether to show a certificate notification
+     * @param showNotification  whether to show a certificate notification (caller must check notification permissions before passing *true*)
      *
      * @throws IllegalArgumentException  when both [launchActivity] and [showNotification] are *false*
      */
+    @SuppressLint("MissingPermission")
     internal fun requestDecision(cert: X509Certificate, launchActivity: Boolean, showNotification: Boolean) {
         if (!launchActivity && !showNotification)
             throw IllegalArgumentException("User decision requires certificate Activity and/or notification")

diff --git a/sample-app/build.gradle b/sample-app/build.gradle
@@ -52,9 +52,9 @@ dependencies {
     implementation 'androidx.lifecycle:lifecycle-runtime-ktx:2.6.2'
 
     implementation 'androidx.appcompat:appcompat:1.6.1'
-    implementation 'com.google.android.material:material:1.9.0'
+    implementation 'com.google.android.material:material:1.10.0'
 
-    implementation 'androidx.activity:activity-compose:1.7.2'
+    implementation 'androidx.activity:activity-compose:1.8.0'
     implementation platform('androidx.compose:compose-bom:2023.08.00')
     implementation 'androidx.compose.material:material'
     implementation 'androidx.compose.ui:ui'

diff --git a/sample-app/src/main/java/at/bitfire/cert4android/demo/MainActivity.kt b/sample-app/src/main/java/at/bitfire/cert4android/demo/MainActivity.kt
@@ -1,6 +1,7 @@
 package at.bitfire.cert4android.demo
 
 import android.Manifest
+import android.annotation.SuppressLint
 import android.app.Application
 import android.content.pm.PackageManager
 import android.net.SSLCertificateSocketFactory
@@ -15,12 +16,7 @@ import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.rememberScrollState
 import androidx.compose.foundation.verticalScroll
-import androidx.compose.material.Button
-import androidx.compose.material.Checkbox
-import androidx.compose.material.MaterialTheme
-import androidx.compose.material.SnackbarHost
-import androidx.compose.material.SnackbarHostState
-import androidx.compose.material.Text
+import androidx.compose.material.*
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.livedata.observeAsState
@@ -34,11 +30,10 @@ import androidx.lifecycle.viewModelScope
 import at.bitfire.cert4android.Cert4Android
 import at.bitfire.cert4android.CustomCertManager
 import at.bitfire.cert4android.CustomCertStore
-import at.bitfire.cert4android.CustomHostnameVerifier
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.launch
-import org.apache.http.conn.ssl.BrowserCompatHostnameVerifier
+import org.apache.http.conn.ssl.AllowAllHostnameVerifier
 import org.apache.http.conn.ssl.StrictHostnameVerifier
 import java.net.URL
 import javax.net.ssl.HttpsURLConnection
@@ -67,37 +62,37 @@ class MainActivity : ComponentActivity() {
                     }
 
                     Button(onClick = {
-                        model.testAccess()
+                        model.testAccess("https://www.github.com")
                     }, modifier = Modifier.padding(top = 16.dp)) {
                         Text("Access normal URL with trusted system certs")
                     }
 
                     Button(onClick = {
-                        model.testAccess(trustSystemCerts = false)
+                        model.testAccess("https://www.github.com", trustSystemCerts = false)
                     }, modifier = Modifier.padding(top = 16.dp)) {
                         Text("Access normal URL with distrusted system certs")
                     }
 
                     Button(onClick = {
-                        model.testAccess(url = "https://expired.badssl.com/")
+                        model.testAccess("https://expired.badssl.com/")
                     }, modifier = Modifier.padding(top = 16.dp)) {
                         Text("Access URL with expired certificate")
                     }
 
                     Button(onClick = {
-                        model.testAccess(url = "https://self-signed.badssl.com/")
+                        model.testAccess("https://self-signed.badssl.com/")
                     }, modifier = Modifier.padding(top = 16.dp)) {
                         Text("Access URL with self-signed certificate")
                     }
 
                     Button(onClick = {
-                        model.testAccess(url = "https://wrong.host.badssl.com/")
+                        model.testAccess("https://wrong.host.badssl.com/")
                     }, modifier = Modifier.padding(top = 16.dp)) {
                         Text("Access URL with certificate for wrong host name")
                     }
 
                     Button(onClick = {
-                        model.testAccess(url = "https://wrong.host.badssl.com/", trustSystemCerts = false)
+                        model.testAccess("https://wrong.host.badssl.com/", trustSystemCerts = false)
                     }, modifier = Modifier.padding(top = 16.dp)) {
                         Text("Access URL with certificate for wrong host name with distrusted system certs")
                     }
@@ -124,13 +119,15 @@ class MainActivity : ComponentActivity() {
     }
 
 
+    @SuppressLint("AllowAllHostnameVerifier")
     class Model(application: Application): AndroidViewModel(application) {
 
         val appInForeground = MutableStateFlow(true)
         val resultMessage = MutableLiveData<String>()
 
         init {
-            HttpsURLConnection.setDefaultHostnameVerifier(CustomHostnameVerifier(getApplication(), StrictHostnameVerifier()))
+            // The default HostnameVerifier is called before our per-connection HostnameVerifier.
+            HttpsURLConnection.setDefaultHostnameVerifier(AllowAllHostnameVerifier())
         }
 
         fun reset() = viewModelScope.launch(Dispatchers.IO) {
@@ -141,7 +138,7 @@ class MainActivity : ComponentActivity() {
             appInForeground.value = foreground
         }
 
-        fun testAccess(url: String = "https://www.github.com", trustSystemCerts: Boolean = true) = viewModelScope.launch(Dispatchers.IO) {
+        fun testAccess(url: String, trustSystemCerts: Boolean = true) = viewModelScope.launch(Dispatchers.IO) {
             try {
                 val urlConn = URL(url).openConnection() as HttpsURLConnection
 
@@ -151,6 +148,7 @@ class MainActivity : ComponentActivity() {
                     trustSystemCerts = trustSystemCerts,
                     appInForeground = appInForeground
                 )
+                urlConn.hostnameVerifier = certMgr.HostnameVerifier(StrictHostnameVerifier())
                 urlConn.sslSocketFactory = object : SSLCertificateSocketFactory(1000) {
                     init {
                         setTrustManagers(arrayOf(certMgr))