Fix Conscrypt crash on accepting/rejecting certain certificates (#48)

* Make Conscrypt initialization explicit

* Refactor Conscrypt initialization and improve test structure. Add test for crashing X509Certificate.toString().

* Downgrade Conscrypt version to 2.5.2 until issue #1268 is resolved

* Fix unit tests

* Fix unit tests

* Explicitly provide crashing expired certificate and minor changes

* CustomCertStore: find certificate alias instead of relying on tag for removal

Author: rfc2822

gradle/libs.versions.toml

@@ -7,7 +7,8 @@ androidx-lifecycle = "2.8.7"
 androidx-test-core = "1.6.1"
 androidx-test-runner = "1.6.2"
 androidx-test-rules = "1.6.1"
-conscrypt = "2.5.3"
+# don't update to 2.5.3 / until https://github.com/google/conscrypt/issues/1268 is fixed
+conscrypt = "2.5.2"
 compose-bom = "2025.01.01"
 dokka = "1.9.20"
 junit = "4.13.2"

lib/src/androidTest/java/at/bitfire/cert4android/ConscryptTest.kt

@@ -0,0 +1,63 @@
+package at.bitfire.cert4android
+
+import org.junit.Before
+import org.junit.Test
+import java.security.cert.CertificateFactory
+import java.security.cert.X509Certificate
+
+class ConscryptTest {
+
+    @Before
+    fun setUp() {
+        ConscryptIntegration.initialize()
+    }
+
+
+    @Test
+    fun test_X509Certificate_toString() {
+        val certFactory = CertificateFactory.getInstance("X.509")
+        val testCert = certFactory.generateCertificate(RAW_EXPIRED_CERT.byteInputStream()) as X509Certificate
+
+        // Crashes with Conscrypt 2.5.3
+        System.err.println(testCert.toString())
+    }
+
+
+    companion object {
+
+        const val RAW_EXPIRED_CERT = "-----BEGIN CERTIFICATE-----\n" +
+                "MIIFdDCCBFygAwIBAgIQJ2buVutJ846r13Ci/ITeIjANBgkqhkiG9w0BAQwFADBv\n" +
+                "MQswCQYDVQQGEwJTRTEUMBIGA1UEChMLQWRkVHJ1c3QgQUIxJjAkBgNVBAsTHUFk\n" +
+                "ZFRydXN0IEV4dGVybmFsIFRUUCBOZXR3b3JrMSIwIAYDVQQDExlBZGRUcnVzdCBF\n" +
+                "eHRlcm5hbCBDQSBSb290MB4XDTAwMDUzMDEwNDgzOFoXDTIwMDUzMDEwNDgzOFow\n" +
+                "gYUxCzAJBgNVBAYTAkdCMRswGQYDVQQIExJHcmVhdGVyIE1hbmNoZXN0ZXIxEDAO\n" +
+                "BgNVBAcTB1NhbGZvcmQxGjAYBgNVBAoTEUNPTU9ETyBDQSBMaW1pdGVkMSswKQYD\n" +
+                "VQQDEyJDT01PRE8gUlNBIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MIICIjANBgkq\n" +
+                "hkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAkehUktIKVrGsDSTdxc9EZ3SZKzejfSNw\n" +
+                "AHG8U9/E+ioSj0t/EFa9n3Byt2F/yUsPF6c947AEYe7/EZfH9IY+Cvo+XPmT5jR6\n" +
+                "2RRr55yzhaCCenavcZDX7P0N+pxs+t+wgvQUfvm+xKYvT3+Zf7X8Z0NyvQwA1onr\n" +
+                "ayzT7Y+YHBSrfuXjbvzYqOSSJNpDa2K4Vf3qwbxstovzDo2a5JtsaZn4eEgwRdWt\n" +
+                "4Q08RWD8MpZRJ7xnw8outmvqRsfHIKCxH2XeSAi6pE6p8oNGN4Tr6MyBSENnTnIq\n" +
+                "m1y9TBsoilwie7SrmNnu4FGDwwlGTm0+mfqVF9p8M1dBPI1R7Qu2XK8sYxrfV8g/\n" +
+                "vOldxJuvRZnio1oktLqpVj3Pb6r/SVi+8Kj/9Lit6Tf7urj0Czr56ENCHonYhMsT\n" +
+                "8dm74YlguIwoVqwUHZwK53Hrzw7dPamWoUi9PPevtQ0iTMARgexWO/bTouJbt7IE\n" +
+                "IlKVgJNp6I5MZfGRAy1wdALqi2cVKWlSArvX31BqVUa/oKMoYX9w0MOiqiwhqkfO\n" +
+                "KJwGRXa/ghgntNWutMtQ5mv0TIZxMOmm3xaG4Nj/QN370EKIf6MzOi5cHkERgWPO\n" +
+                "GHFrK+ymircxXDpqR+DDeVnWIBqv8mqYqnK8V0rSS527EPywTEHl7R09XiidnMy/\n" +
+                "s1Hap0flhFMCAwEAAaOB9DCB8TAfBgNVHSMEGDAWgBStvZh6NLQm9/rEJlTvA73g\n" +
+                "JMtUGjAdBgNVHQ4EFgQUu69+Aj36pvE8hI6t7jiY7NkyMtQwDgYDVR0PAQH/BAQD\n" +
+                "AgGGMA8GA1UdEwEB/wQFMAMBAf8wEQYDVR0gBAowCDAGBgRVHSAAMEQGA1UdHwQ9\n" +
+                "MDswOaA3oDWGM2h0dHA6Ly9jcmwudXNlcnRydXN0LmNvbS9BZGRUcnVzdEV4dGVy\n" +
+                "bmFsQ0FSb290LmNybDA1BggrBgEFBQcBAQQpMCcwJQYIKwYBBQUHMAGGGWh0dHA6\n" +
+                "Ly9vY3NwLnVzZXJ0cnVzdC5jb20wDQYJKoZIhvcNAQEMBQADggEBAGS/g/FfmoXQ\n" +
+                "zbihKVcN6Fr30ek+8nYEbvFScLsePP9NDXRqzIGCJdPDoCpdTPW6i6FtxFQJdcfj\n" +
+                "Jw5dhHk3QBN39bSsHNA7qxcS1u80GH4r6XnTq1dFDK8o+tDb5VCViLvfhVdpfZLY\n" +
+                "Uspzgb8c8+a4bmYRBbMelC1/kZWSWfFMzqORcUx8Rww7Cxn2obFshj5cqsQugsv5\n" +
+                "B5a6SE2Q8pTIqXOi6wZ7I53eovNNVZ96YUWYGGjHXkBrI/V5eu+MtWuLt29G9Hvx\n" +
+                "PUsE2JOAWVrgQSQdso8VYFhH2+9uRv0V9dlfmrPb2LjkQLPNlzmuhbsdjrzch5vR\n" +
+                "pu/xO28QOG8=\n" +
+                "-----END CERTIFICATE-----\n"
+
+    }
+
+}

lib/src/androidTest/java/at/bitfire/cert4android/CustomCertManagerTest.kt

@@ -12,39 +12,24 @@ import java.io.IOException
 import java.net.URL
 import java.security.cert.CertificateException
 import java.security.cert.X509Certificate
-import javax.net.ssl.HttpsURLConnection
 
 class CustomCertManagerTest {
 
-    companion object {
-        private fun getSiteCertificates(url: URL): List<X509Certificate> {
-            val conn = url.openConnection() as HttpsURLConnection
-            try {
-                conn.inputStream.read()
-                val certs = mutableListOf<X509Certificate>()
-                conn.serverCertificates.forEach { certs += it as X509Certificate }
-                return certs
-            } finally {
-                conn.disconnect()
-            }
-        }
-    }
-
     private val context by lazy { InstrumentationRegistry.getInstrumentation().targetContext }
 
     private lateinit var certManager: CustomCertManager
     private lateinit var paranoidCertManager: CustomCertManager
 
-    private var siteCerts: List<X509Certificate>? = null
-    init {
+    private var siteCerts: List<X509Certificate>? =
         try {
-            siteCerts = getSiteCertificates(URL("https://www.davx5.com"))
+            TestCertificates.getSiteCertificates(URL("https://www.davx5.com"))
         } catch(_: IOException) {
+            null
         }
-        assumeNotNull(siteCerts)
+    init {
+        assumeNotNull("Couldn't load certificate from Web", siteCerts)
     }
 
-
     @Before
     fun createCertManager() {
         certManager = CustomCertManager(context, true, null)
@@ -85,6 +70,8 @@ class CustomCertManagerTest {
     }
 
 
+    // helpers
+
     private fun addTrustedCertificate() {
         CustomCertStore.getInstance(context).setTrustedByUser(siteCerts!!.first())
     }
@@ -93,4 +80,4 @@ class CustomCertManagerTest {
         CustomCertStore.getInstance(context).setUntrustedByUser(siteCerts!!.first())
     }
 
-}
+}

lib/src/androidTest/java/at/bitfire/cert4android/CustomCertStoreTest.kt

@@ -8,16 +8,13 @@ import androidx.test.platform.app.InstrumentationRegistry
 import org.junit.Assert.*
 import org.junit.Before
 import org.junit.Test
-import java.security.cert.CertificateFactory
-import java.security.cert.X509Certificate
 
 class CustomCertStoreTest {
 
     val context = InstrumentationRegistry.getInstrumentation().targetContext
     val certStore = CustomCertStore.getInstance(context)
 
-    val testCert = TestCertificate.testCert
-
+    val testCert = TestCertificates.testCert
 
     @Before
     fun clearKeys() {
@@ -41,8 +38,9 @@ class CustomCertStoreTest {
         // test whether cert was stored to disk:
         // create another cert store to make sure data is loaded from disk again
         val anotherCertStore = CustomCertStore(context)
-        assertEquals("4694b8d19ee1b087a21aa2383dddf7bc82baf4fc19ca17c6c064fad45a37085c8404073ccbef7736423797a6ddfc49fc500906f3b9c4eaab01c5cf4b79d16169b75c82c81fac437cbea36186e72e2f21cf3f30a2f82dac0091e0006a0a120adfae6bdd8c26146b54c99e0e7d9f95b9abc4a69987ff004a247acf2d17c0ee6774ce4d7b763303c521b699883ca7e89d7e3d89843ccc68a9b1b94c3624a31e14e3ecce0c3b74d061419659fdd7ce9e01b74a3d22244bcb25025079ca5060fd3946fcb9b343fe444aa213c97493268e5148b090df6886e3aac5d9a3d1715afbd435aa6d5f98770908dcdec4c1938820497a6bdb2b17eab4d4bac8ba45a449cb94f5",
-            anotherCertStore.userKeyStore.aliases().nextElement())
+        assertTrue(anotherCertStore.userKeyStore.aliases().toList().any { alias ->
+            anotherCertStore.isTrustedByUser(testCert)
+        })
     }
 
     @Test
@@ -59,5 +57,4 @@ class CustomCertStoreTest {
         assertFalse(anotherCertStore.isTrustedByUser(testCert))
     }
 
-
 }

lib/src/androidTest/java/at/bitfire/cert4android/TestCertificate.kt

@@ -0,0 +1,78 @@
+package at.bitfire.cert4android
+
+import android.net.SSLCertificateSocketFactory
+import org.apache.http.conn.ssl.AllowAllHostnameVerifier
+import java.net.URL
+import java.security.cert.CertificateFactory
+import java.security.cert.X509Certificate
+import javax.net.ssl.HttpsURLConnection
+import javax.net.ssl.X509TrustManager
+
+/**
+ * Provides certificates for testing.
+ */
+object TestCertificates {
+
+    init {
+        ConscryptIntegration.initialize()
+    }
+
+    val certFactory = CertificateFactory.getInstance("X.509")
+
+    const val RAW_TEST_CERT = "-----BEGIN CERTIFICATE-----\n" +
+            "MIICxzCCAa+gAwIBAgIUe7x8TfMqQlJ+qTF/L+n6NqRqKAwwDQYJKoZIhvcNAQEL\n" +
+            "BQAwDjEMMAoGA1UEAwwDdG50MB4XDTIwMDIxODE5NTYyMFoXDTMwMDIxNTE5NTYy\n" +
+            "MFowDjEMMAoGA1UEAwwDdG50MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC\n" +
+            "AQEAzqBnVAWwIp8oOrcEJzplOLzd8ZKscmrJ0oxMf7oiYUE5+D3I1IjsCIeUdwfH\n" +
+            "9zDKmNo3lwyFUTjw2WotssawmEH0LGfabi6h/1bbFG4QOQC7KYMtD8tqzR6F0WVe\n" +
+            "oZ5uM5VvQB+dRvlq2CdWb8NcBkyd2pQ8RvJsft3fWY5ix3CL+OZ8lXOGqkxN780m\n" +
+            "RkrxdYog3fvWC1CMSix8Y28q4JwRRAMP0JhBGIdFpnEPLowh6HhhPRiwAn+ksSey\n" +
+            "Rz39AsUErUUCD7soCsDSzu80uieF9enEVweqnn/ayPhJlX0Drw7UwrC88UqdLqRD\n" +
+            "Da/ucJYzKkMgHZJ7EXNh2WZpbwIDAQABox0wGzAJBgNVHRMEAjAAMA4GA1UdEQQH\n" +
+            "MAWCA3RudDANBgkqhkiG9w0BAQsFAAOCAQEARpS40Z7hsIeiGqI4Pd33vIK69PwZ\n" +
+            "yhfGwGT61Fo3CFyEBAc8y+93NkI3l6bd/En8UAkG87nE6qsBxc9LedFhabdcgsgf\n" +
+            "rEN8vqNhhucuLyHPPzCi+C2sAJHgAGoKEgrfrmvdjCYUa1TJng59n5W5q8SmmYf/\n" +
+            "AEokes8tF8DuZ3TOTXt2MwPFIbaZiDyn6J1+PYmEPMxoqbG5TDYkox4U4+zODDt0\n" +
+            "0GFBlln9186eAbdKPSIkS8slAlB5ylBg/TlG/LmzQ/5ESqITyXSTJo5RSLCQ32iG\n" +
+            "46rF2aPRcVr71DWqbV+YdwkI3N7EwZOIIEl6a9srF+q01LrIukWkScuU9Q==\n" +
+            "-----END CERTIFICATE-----\n"
+
+    /** some test certificate (untrusted Snakeoil certificate generated by Debian) */
+    val testCert = certFactory.generateCertificate(RAW_TEST_CERT.byteInputStream()) as X509Certificate
+
+
+    /**
+     * Get the certificates of a site (bypassing all trusted checks).
+     *
+     * @param url the URL to get the certificates from
+     * @return the certificates of the site
+     */
+    fun getSiteCertificates(url: URL): List<X509Certificate> {
+        val conn = url.openConnection() as HttpsURLConnection
+        try {
+            conn.hostnameVerifier = AllowAllHostnameVerifier()
+            conn.sslSocketFactory = object : SSLCertificateSocketFactory(1000) {
+                init {
+                    setTrustManagers(arrayOf(object : X509TrustManager {
+                        override fun checkClientTrusted(
+                            chain: Array<out X509Certificate?>?,
+                            authType: String?
+                        ) { /* OK */ }
+                        override fun checkServerTrusted(
+                            chain: Array<out X509Certificate?>?,
+                            authType: String?
+                        ) { /* OK */ }
+                        override fun getAcceptedIssuers(): Array<out X509Certificate?>? = emptyArray()
+                    }))
+                }
+            }
+            conn.inputStream.read()
+            val certs = mutableListOf<X509Certificate>()
+            conn.serverCertificates.forEach { certs += it as X509Certificate }
+            return certs
+        } finally {
+            conn.disconnect()
+        }
+    }
+
+}

lib/src/androidTest/java/at/bitfire/cert4android/TestCertificates.kt

@@ -22,7 +22,7 @@ class UserDecisionRegistryTest {
     private val certStore = CustomCertStore.getInstance(context)
     private val registry = UserDecisionRegistry.getInstance(context)
 
-    private val testCert = TestCertificate.testCert
+    private val testCert = TestCertificates.testCert
 
 
     @Before

lib/src/androidTest/java/at/bitfire/cert4android/UserDecisionRegistryTest.kt

@@ -15,11 +15,6 @@ import javax.net.ssl.X509TrustManager
 
 object CertUtils {
 
-    fun fingerprint(cert: X509Certificate, algorithm: String): String {
-        val md = MessageDigest.getInstance(algorithm)
-        return hexString(md.digest(cert.encoded))
-    }
-
     fun getTrustManager(keyStore: KeyStore?): X509TrustManager? {
         try {
             val tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm())
@@ -33,21 +28,23 @@ object CertUtils {
         return null
     }
 
-    fun getTag(cert: X509Certificate): String {
-        val str = StringBuilder()
-        for (b in cert.signature)
-            str.append(String.format(Locale.ROOT, "%02x", b))
-        return str.toString()
+
+    fun getTag(cert: X509Certificate) =
+        fingerprint(cert, "SHA-512", separator = null)
+
+    fun fingerprint(cert: X509Certificate, algorithm: String, separator: Char? = ':'): String {
+        val md = MessageDigest.getInstance(algorithm)
+        return hexString(md.digest(cert.encoded), separator)
     }
 
-    fun hexString(data: ByteArray): String {
+    fun hexString(data: ByteArray, separator: Char? = ':'): String {
         val str = StringBuilder()
         for ((idx, b) in data.withIndex()) {
-            if (idx != 0)
-                str.append(':')
+            if (idx != 0 && separator != null)
+                str.append(separator)
             str.append(String.format("%02x", b).uppercase(Locale.ROOT))
         }
         return str.toString()
     }
 
-}
+}

lib/src/main/java/at/bitfire/cert4android/CertUtils.kt

@@ -0,0 +1,29 @@
+package at.bitfire.cert4android
+
+import org.conscrypt.Conscrypt
+import java.security.Security
+import javax.net.ssl.SSLContext
+
+object ConscryptIntegration {
+
+    private var initialized = false
+
+    @Synchronized
+    fun initialize() {
+        if (initialized)
+            return
+
+        // initialize Conscrypt
+        Security.insertProviderAt(Conscrypt.newProvider(), 1)
+
+        val version = Conscrypt.version()
+        Cert4Android.log.info("Using Conscrypt/${version.major()}.${version.minor()}.${version.patch()} for TLS")
+
+        val engine = SSLContext.getDefault().createSSLEngine()
+        Cert4Android.log.info("Enabled protocols: ${engine.enabledProtocols.joinToString(", ")}")
+        Cert4Android.log.info("Enabled ciphers: ${engine.enabledCipherSuites.joinToString(", ")}")
+
+        initialized = true
+    }
+
+}

lib/src/main/java/at/bitfire/cert4android/ConscryptIntegration.kt

@@ -15,6 +15,8 @@ import javax.net.ssl.X509TrustManager
 /**
  * TrustManager to handle custom certificates.
  *
+ * Initializes Conscrypt when it is first loaded.
+ *
  * @param trustSystemCerts whether system certificates will be trusted
  * @param appInForeground  - `true`: if needed, directly launches [TrustCertificateActivity] and shows notification (if possible)
  *                         - `false`: if needed, shows notification (if possible)
@@ -78,4 +80,14 @@ class CustomCertManager @JvmOverloads constructor(
 
     }
 
+
+    companion object {
+
+        init {
+            // On first load of this class, initialize Conscrypt.
+            ConscryptIntegration.initialize()
+        }
+
+    }
+
 }