From f86fa6cb5d9ddb32c3762d7da8de61f3d5922395 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 1 Jul 2020 09:48:03 +0200 Subject: [PATCH 1/3] Avoid Exception if array is empty. --- .../im/vector/matrix/android/internal/network/ssl/CertUtil.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/CertUtil.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/CertUtil.kt index 2346ff8877..36e40a16c7 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/CertUtil.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/CertUtil.kt @@ -239,12 +239,12 @@ internal object CertUtil { fun newConnectionSpecs(hsConfig: HomeServerConnectionConfig): List { 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()) } From b8b79de91c62d8748a319d1621067a1f930d369a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 1 Jul 2020 11:54:03 +0200 Subject: [PATCH 2/3] PinnedTrustManager differ for API 24+ --- .../android/internal/network/ssl/CertUtil.kt | 2 +- .../network/ssl/PinnedTrustManager.kt | 28 ++- .../network/ssl/PinnedTrustManagerApi24.kt | 163 ++++++++++++++++++ .../network/ssl/PinnedTrustManagerProvider.kt | 41 +++++ 4 files changed, 217 insertions(+), 17 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerApi24.kt create mode 100644 matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerProvider.kt diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/CertUtil.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/CertUtil.kt index 36e40a16c7..4945afa93c 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/CertUtil.kt +++ b/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(PinnedTrustManager(hsConfig.allowedFingerprints, defaultTrustManager)) + val trustPinned = arrayOf(PinnedTrustManagerProvider.provide(hsConfig.allowedFingerprints, defaultTrustManager)) val sslSocketFactory: SSLSocketFactory diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManager.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManager.kt index de41d168fd..73bc3c7d57 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManager.kt +++ b/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?, +internal class PinnedTrustManager(private val fingerprints: List, 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, 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?, @Throws(CertificateException::class) override fun checkServerTrusted(chain: Array, 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` ? */) } } @@ -76,12 +73,11 @@ internal class PinnedTrustManager(private val fingerprints: List?, val cert = chain[0] var found = false - if (fingerprints != null) { - for (allowedFingerprint in fingerprints) { - if (allowedFingerprint.matchesCert(cert)) { - found = true - break - } + + for (allowedFingerprint in fingerprints) { + if (allowedFingerprint.matchesCert(cert)) { + found = true + break } } @@ -91,6 +87,6 @@ internal class PinnedTrustManager(private val fingerprints: List?, } override fun getAcceptedIssuers(): Array { - return emptyArray() + return defaultTrustManager?.acceptedIssuers ?: emptyArray() } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerApi24.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerApi24.kt new file mode 100644 index 0000000000..4cdae14f25 --- /dev/null +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerApi24.kt @@ -0,0 +1,163 @@ +/* + * 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, + private val defaultTrustManager: X509ExtendedTrustManager?) : X509ExtendedTrustManager() { + + @Throws(CertificateException::class) + override fun checkClientTrusted(chain: Array, 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, 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, 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, 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, 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, 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) { + val cert = chain[0] + + var found = false + for (allowedFingerprint in fingerprints) { + if (allowedFingerprint.matchesCert(cert)) { + found = true + break + } + } + + if (!found) { + throw UnrecognizedCertificateException(cert, Fingerprint.newSha256Fingerprint(cert), null) + } + } + + override fun getAcceptedIssuers(): Array { + return defaultTrustManager?.acceptedIssuers ?: emptyArray() + } +} diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerProvider.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerProvider.kt new file mode 100644 index 0000000000..d47adbfd07 --- /dev/null +++ b/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?, + 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 } + ) + } + } +} From 057f6fdf269fcb32b5f532c947217585ccf02b2c Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 1 Jul 2020 12:11:45 +0200 Subject: [PATCH 3/3] Kotlin style --- .../internal/network/ssl/PinnedTrustManager.kt | 11 +---------- .../internal/network/ssl/PinnedTrustManagerApi24.kt | 10 +--------- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManager.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManager.kt index 73bc3c7d57..615358310f 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManager.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManager.kt @@ -72,16 +72,7 @@ internal class PinnedTrustManager(private val fingerprints: List, private fun checkTrusted(chain: Array) { val cert = chain[0] - var found = false - - for (allowedFingerprint in fingerprints) { - if (allowedFingerprint.matchesCert(cert)) { - found = true - break - } - } - - if (!found) { + if (!fingerprints.any { it.matchesCert(cert) }) { throw UnrecognizedCertificateException(cert, Fingerprint.newSha256Fingerprint(cert), null) } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerApi24.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerApi24.kt index 4cdae14f25..98257caefc 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerApi24.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/ssl/PinnedTrustManagerApi24.kt @@ -144,15 +144,7 @@ internal class PinnedTrustManagerApi24(private val fingerprints: List) { val cert = chain[0] - var found = false - for (allowedFingerprint in fingerprints) { - if (allowedFingerprint.matchesCert(cert)) { - found = true - break - } - } - - if (!found) { + if (!fingerprints.any { it.matchesCert(cert) }) { throw UnrecognizedCertificateException(cert, Fingerprint.newSha256Fingerprint(cert), null) } }