Merge pull request #1588 from vector-im/feature/fixes_before_beta_release

Fix issue with not trusted certificate on API 24+

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/CertUtil.kt

@@ -175,7 +175,7 @@ internal object CertUtil {
                 }
             }
 
-            val trustPinned = arrayOf<TrustManager>(PinnedTrustManager(hsConfig.allowedFingerprints, defaultTrustManager))
+            val trustPinned = arrayOf<TrustManager>(PinnedTrustManagerProvider.provide(hsConfig.allowedFingerprints, defaultTrustManager))
 
             val sslSocketFactory: SSLSocketFactory
 
@@ -239,12 +239,12 @@ internal object CertUtil {
     fun newConnectionSpecs(hsConfig: HomeServerConnectionConfig): List<ConnectionSpec> {
         val builder = ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
         val tlsVersions = hsConfig.tlsVersions
-        if (null != tlsVersions) {
+        if (null != tlsVersions && tlsVersions.isNotEmpty()) {
             builder.tlsVersions(*tlsVersions.toTypedArray())
         }
 
         val tlsCipherSuites = hsConfig.tlsCipherSuites
-        if (null != tlsCipherSuites) {
+        if (null != tlsCipherSuites && tlsCipherSuites.isNotEmpty()) {
             builder.cipherSuites(*tlsCipherSuites.toTypedArray())
         }
 

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManager.kt

@@ -27,26 +27,23 @@ import javax.net.ssl.X509TrustManager
  */
 
 /**
- * @param fingerprints        An array of SHA256 cert fingerprints
+ * @param fingerprints        Not empty array of SHA256 cert fingerprints
  * @param defaultTrustManager Optional trust manager to fall back on if cert does not match
  * any of the fingerprints. Can be null.
  */
-internal class PinnedTrustManager(private val fingerprints: List<Fingerprint>?,
+internal class PinnedTrustManager(private val fingerprints: List<Fingerprint>,
                                   private val defaultTrustManager: X509TrustManager?) : X509TrustManager {
 
-    // Set to false to perform some test
-    private val USE_DEFAULT_TRUST_MANAGER = true
-
     @Throws(CertificateException::class)
     override fun checkClientTrusted(chain: Array<X509Certificate>, s: String) {
         try {
-            if (defaultTrustManager != null && USE_DEFAULT_TRUST_MANAGER) {
+            if (defaultTrustManager != null) {
                 defaultTrustManager.checkClientTrusted(chain, s)
                 return
             }
         } catch (e: CertificateException) {
             // If there is an exception we fall back to checking fingerprints
-            if (fingerprints.isNullOrEmpty()) {
+            if (fingerprints.isEmpty()) {
                 throw UnrecognizedCertificateException(chain[0], Fingerprint.newSha256Fingerprint(chain[0]), e.cause)
             }
         }
@@ -57,13 +54,13 @@ internal class PinnedTrustManager(private val fingerprints: List<Fingerprint>?,
     @Throws(CertificateException::class)
     override fun checkServerTrusted(chain: Array<X509Certificate>, s: String) {
         try {
-            if (defaultTrustManager != null && USE_DEFAULT_TRUST_MANAGER) {
+            if (defaultTrustManager != null) {
                 defaultTrustManager.checkServerTrusted(chain, s)
                 return
             }
         } catch (e: CertificateException) {
             // If there is an exception we fall back to checking fingerprints
-            if (fingerprints == null || fingerprints.isEmpty()) {
+            if (fingerprints.isEmpty()) {
                 throw UnrecognizedCertificateException(chain[0], Fingerprint.newSha256Fingerprint(chain[0]), e.cause /* BMA: Shouldn't be `e` ? */)
             }
         }
@@ -75,22 +72,12 @@ internal class PinnedTrustManager(private val fingerprints: List<Fingerprint>?,
     private fun checkTrusted(chain: Array<X509Certificate>) {
         val cert = chain[0]
 
-        var found = false
+        if (!fingerprints.any { it.matchesCert(cert) }) {
-        if (fingerprints != null) {
-            for (allowedFingerprint in fingerprints) {
-                if (allowedFingerprint.matchesCert(cert)) {
-                    found = true
-                    break
-                }
-            }
-        }
-
-        if (!found) {
             throw UnrecognizedCertificateException(cert, Fingerprint.newSha256Fingerprint(cert), null)
         }
     }
 
     override fun getAcceptedIssuers(): Array<X509Certificate> {
-        return emptyArray()
+        return defaultTrustManager?.acceptedIssuers ?: emptyArray()
     }
 }

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerApi24.kt

@@ -0,0 +1,155 @@
+/*
+ * Copyright (c) 2020 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.matrix.android.internal.network.ssl
+
+import android.os.Build
+import androidx.annotation.RequiresApi
+import java.net.Socket
+import java.security.cert.CertificateException
+import java.security.cert.X509Certificate
+import javax.net.ssl.SSLEngine
+import javax.net.ssl.X509ExtendedTrustManager
+
+/**
+ * Implements a TrustManager that checks Certificates against an explicit list of known
+ * fingerprints.
+ */
+
+/**
+ * @param fingerprints        An array of SHA256 cert fingerprints
+ * @param defaultTrustManager Optional trust manager to fall back on if cert does not match
+ * any of the fingerprints. Can be null.
+ */
+@RequiresApi(Build.VERSION_CODES.N)
+internal class PinnedTrustManagerApi24(private val fingerprints: List<Fingerprint>,
+                                       private val defaultTrustManager: X509ExtendedTrustManager?) : X509ExtendedTrustManager() {
+
+    @Throws(CertificateException::class)
+    override fun checkClientTrusted(chain: Array<X509Certificate>, authType: String, engine: SSLEngine?) {
+        try {
+            if (defaultTrustManager != null) {
+                defaultTrustManager.checkClientTrusted(chain, authType, engine)
+                return
+            }
+        } catch (e: CertificateException) {
+            // If there is an exception we fall back to checking fingerprints
+            if (fingerprints.isEmpty()) {
+                throw UnrecognizedCertificateException(chain[0], Fingerprint.newSha256Fingerprint(chain[0]), e.cause)
+            }
+        }
+
+        checkTrusted(chain)
+    }
+
+    @Throws(CertificateException::class)
+    override fun checkClientTrusted(chain: Array<X509Certificate>, authType: String, socket: Socket?) {
+        try {
+            if (defaultTrustManager != null) {
+                defaultTrustManager.checkClientTrusted(chain, authType, socket)
+                return
+            }
+        } catch (e: CertificateException) {
+            // If there is an exception we fall back to checking fingerprints
+            if (fingerprints.isEmpty()) {
+                throw UnrecognizedCertificateException(chain[0], Fingerprint.newSha256Fingerprint(chain[0]), e.cause)
+            }
+        }
+
+        checkTrusted(chain)
+    }
+
+    @Throws(CertificateException::class)
+    override fun checkClientTrusted(chain: Array<X509Certificate>, authType: String) {
+        try {
+            if (defaultTrustManager != null) {
+                defaultTrustManager.checkClientTrusted(chain, authType)
+                return
+            }
+        } catch (e: CertificateException) {
+            // If there is an exception we fall back to checking fingerprints
+            if (fingerprints.isEmpty()) {
+                throw UnrecognizedCertificateException(chain[0], Fingerprint.newSha256Fingerprint(chain[0]), e.cause)
+            }
+        }
+
+        checkTrusted(chain)
+    }
+
+    @Throws(CertificateException::class)
+    override fun checkServerTrusted(chain: Array<X509Certificate>, authType: String, socket: Socket?) {
+        try {
+            if (defaultTrustManager != null) {
+                defaultTrustManager.checkServerTrusted(chain, authType, socket)
+                return
+            }
+        } catch (e: CertificateException) {
+            // If there is an exception we fall back to checking fingerprints
+            if (fingerprints.isEmpty()) {
+                throw UnrecognizedCertificateException(chain[0], Fingerprint.newSha256Fingerprint(chain[0]), e.cause /* BMA: Shouldn't be `e` ? */)
+            }
+        }
+
+        checkTrusted(chain)
+    }
+
+    @Throws(CertificateException::class)
+    override fun checkServerTrusted(chain: Array<X509Certificate>, authType: String, engine: SSLEngine?) {
+        try {
+            if (defaultTrustManager != null) {
+                defaultTrustManager.checkServerTrusted(chain, authType, engine)
+                return
+            }
+        } catch (e: CertificateException) {
+            // If there is an exception we fall back to checking fingerprints
+            if (fingerprints.isEmpty()) {
+                throw UnrecognizedCertificateException(chain[0], Fingerprint.newSha256Fingerprint(chain[0]), e.cause /* BMA: Shouldn't be `e` ? */)
+            }
+        }
+
+        checkTrusted(chain)
+    }
+
+    @Throws(CertificateException::class)
+    override fun checkServerTrusted(chain: Array<X509Certificate>, s: String) {
+        try {
+            if (defaultTrustManager != null) {
+                defaultTrustManager.checkServerTrusted(chain, s)
+                return
+            }
+        } catch (e: CertificateException) {
+            // If there is an exception we fall back to checking fingerprints
+            if (fingerprints.isEmpty()) {
+                throw UnrecognizedCertificateException(chain[0], Fingerprint.newSha256Fingerprint(chain[0]), e.cause /* BMA: Shouldn't be `e` ? */)
+            }
+        }
+
+        checkTrusted(chain)
+    }
+
+    @Throws(CertificateException::class)
+    private fun checkTrusted(chain: Array<X509Certificate>) {
+        val cert = chain[0]
+
+        if (!fingerprints.any { it.matchesCert(cert) }) {
+            throw UnrecognizedCertificateException(cert, Fingerprint.newSha256Fingerprint(cert), null)
+        }
+    }
+
+    override fun getAcceptedIssuers(): Array<X509Certificate> {
+        return defaultTrustManager?.acceptedIssuers ?: emptyArray()
+    }
+}

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerProvider.kt

@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2020 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.matrix.android.internal.network.ssl
+
+import android.os.Build
+import javax.net.ssl.X509ExtendedTrustManager
+import javax.net.ssl.X509TrustManager
+
+internal object PinnedTrustManagerProvider {
+    // Set to false to perform some tests
+    private const val USE_DEFAULT_TRUST_MANAGER = true
+
+    fun provide(fingerprints: List<Fingerprint>?,
+                defaultTrustManager: X509TrustManager?): X509TrustManager {
+        return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && defaultTrustManager is X509ExtendedTrustManager) {
+            PinnedTrustManagerApi24(
+                    fingerprints.orEmpty(),
+                    defaultTrustManager.takeIf { USE_DEFAULT_TRUST_MANAGER }
+            )
+        } else {
+            PinnedTrustManager(
+                    fingerprints.orEmpty(),
+                    defaultTrustManager.takeIf { USE_DEFAULT_TRUST_MANAGER }
+            )
+        }
+    }
+}