From b9b43664ce455ecad8db1531a6e5e68f43e6fe45 Mon Sep 17 00:00:00 2001
From: Janne Koschinski <janne@kuschku.de>
Date: Wed, 13 May 2020 23:42:09 +0200
Subject: [PATCH] Use system hostname verifier
---
.../quasseldroid/service/QuasselService.kt | 2 +-
.../ssl/QuasselHostnameVerifier.kt | 21 ++-----
.../ssl/custom/QuasselHostnameManager.kt | 14 +++--
.../quasseldroid/ui/chat/ChatActivity.kt | 6 +-
.../libquassel/connection/CoreConnection.kt | 5 +-
.../connection/QuasselSecurityException.kt | 11 ++--
.../de/kuschku/libquassel/session/Session.kt | 5 +-
.../session/manager/ConnectionInfo.kt | 2 +-
.../ssl/BrowserCompatibleHostnameVerifier.kt | 63 -------------------
.../ssl/TrustAllHostnameVerifier.kt | 28 ---------
.../X509CertificateHelper.kt} | 11 ++--
.../de/kuschku/libquassel/ssl/X509Helper.kt | 6 +-
.../libquassel/util/nio/WrappedChannel.kt | 26 ++++----
13 files changed, 54 insertions(+), 146 deletions(-)
delete mode 100644 lib/src/main/java/de/kuschku/libquassel/ssl/BrowserCompatibleHostnameVerifier.kt
delete mode 100644 lib/src/main/java/de/kuschku/libquassel/ssl/TrustAllHostnameVerifier.kt
rename lib/src/main/java/de/kuschku/libquassel/{connection/HostnameVerifier.kt => ssl/X509CertificateHelper.kt} (74%)
diff --git a/app/src/main/java/de/kuschku/quasseldroid/service/QuasselService.kt b/app/src/main/java/de/kuschku/quasseldroid/service/QuasselService.kt
index 9ae229812..0f7685708 100644
--- a/app/src/main/java/de/kuschku/quasseldroid/service/QuasselService.kt
+++ b/app/src/main/java/de/kuschku/quasseldroid/service/QuasselService.kt
@@ -28,7 +28,6 @@ import androidx.core.app.RemoteInput
import androidx.lifecycle.Observer
import com.github.pwittchen.reactivenetwork.library.rx2.ReactiveNetwork
import de.kuschku.libquassel.connection.ConnectionState
-import de.kuschku.libquassel.connection.HostnameVerifier
import de.kuschku.libquassel.connection.SocketAddress
import de.kuschku.libquassel.protocol.*
import de.kuschku.libquassel.quassel.BufferInfo
@@ -68,6 +67,7 @@ import de.kuschku.quasseldroid.util.ui.LocaleHelper
import io.reactivex.subjects.BehaviorSubject
import org.threeten.bp.Instant
import javax.inject.Inject
+import javax.net.ssl.HostnameVerifier
import javax.net.ssl.X509TrustManager
class QuasselService : DaggerLifecycleService(),
diff --git a/app/src/main/java/de/kuschku/quasseldroid/ssl/QuasselHostnameVerifier.kt b/app/src/main/java/de/kuschku/quasseldroid/ssl/QuasselHostnameVerifier.kt
index 0bc75e3c1..85b30d7a4 100644
--- a/app/src/main/java/de/kuschku/quasseldroid/ssl/QuasselHostnameVerifier.kt
+++ b/app/src/main/java/de/kuschku/quasseldroid/ssl/QuasselHostnameVerifier.kt
@@ -19,25 +19,16 @@
package de.kuschku.quasseldroid.ssl
-import de.kuschku.libquassel.connection.HostnameVerifier
-import de.kuschku.libquassel.connection.QuasselSecurityException
-import de.kuschku.libquassel.connection.SocketAddress
-import de.kuschku.libquassel.ssl.BrowserCompatibleHostnameVerifier
import de.kuschku.quasseldroid.ssl.custom.QuasselHostnameManager
-import java.security.cert.X509Certificate
-import javax.net.ssl.SSLException
+import javax.net.ssl.HostnameVerifier
+import javax.net.ssl.HttpsURLConnection
+import javax.net.ssl.SSLSession
class QuasselHostnameVerifier(
private val hostnameManager: QuasselHostnameManager,
- private val hostnameVerifier: HostnameVerifier = BrowserCompatibleHostnameVerifier()
+ private val hostnameVerifier: HostnameVerifier = HttpsURLConnection.getDefaultHostnameVerifier()
) : HostnameVerifier {
- override fun checkValid(address: SocketAddress, chain: Array<out X509Certificate>) {
- try {
- if (!hostnameManager.isValid(address, chain)) {
- hostnameVerifier.checkValid(address, chain)
- }
- } catch (e: SSLException) {
- throw QuasselSecurityException.Hostname(chain, address, e)
- }
+ override fun verify(hostname: String?, session: SSLSession?): Boolean {
+ return hostnameManager.verify(hostname, session) || hostnameVerifier.verify(hostname, session)
}
}
diff --git a/app/src/main/java/de/kuschku/quasseldroid/ssl/custom/QuasselHostnameManager.kt b/app/src/main/java/de/kuschku/quasseldroid/ssl/custom/QuasselHostnameManager.kt
index 6320434c6..e0b5f2d3f 100644
--- a/app/src/main/java/de/kuschku/quasseldroid/ssl/custom/QuasselHostnameManager.kt
+++ b/app/src/main/java/de/kuschku/quasseldroid/ssl/custom/QuasselHostnameManager.kt
@@ -19,17 +19,19 @@
package de.kuschku.quasseldroid.ssl.custom
-import de.kuschku.libquassel.connection.SocketAddress
+import de.kuschku.libquassel.ssl.toJavaCertificate
import de.kuschku.quasseldroid.persistence.dao.SslHostnameWhitelistDao
import de.kuschku.quasseldroid.util.helper.sha1Fingerprint
-import java.security.cert.X509Certificate
+import javax.net.ssl.HostnameVerifier
+import javax.net.ssl.SSLSession
class QuasselHostnameManager(
private val hostnameWhitelist: SslHostnameWhitelistDao
-) {
- fun isValid(address: SocketAddress, chain: Array<out X509Certificate>): Boolean {
- val leafCertificate = chain.firstOrNull() ?: return false
- val whitelistEntry = hostnameWhitelist.find(leafCertificate.sha1Fingerprint, address.host)
+) : HostnameVerifier {
+ override fun verify(hostname: String?, session: SSLSession?): Boolean {
+ val chain = session?.peerCertificateChain?.toJavaCertificate()
+ val leafCertificate = chain?.firstOrNull() ?: return false
+ val whitelistEntry = hostnameWhitelist.find(leafCertificate.sha1Fingerprint, hostname ?: "")
return whitelistEntry != null
}
}
diff --git a/app/src/main/java/de/kuschku/quasseldroid/ui/chat/ChatActivity.kt b/app/src/main/java/de/kuschku/quasseldroid/ui/chat/ChatActivity.kt
index f36613752..2e117103f 100644
--- a/app/src/main/java/de/kuschku/quasseldroid/ui/chat/ChatActivity.kt
+++ b/app/src/main/java/de/kuschku/quasseldroid/ui/chat/ChatActivity.kt
@@ -526,7 +526,7 @@ class ChatActivity : ServiceBoundActivity(), SharedPreferences.OnSharedPreferenc
.show()
} else {
val leafCertificate = it.certificateChain?.firstOrNull()
- if (leafCertificate == null) {
+ if (leafCertificate == null || it is QuasselSecurityException.NoCertificate) {
// No certificate exists in the chain
MaterialDialog.Builder(this)
.title(R.string.label_error_certificate)
@@ -584,7 +584,7 @@ class ChatActivity : ServiceBoundActivity(), SharedPreferences.OnSharedPreferenc
.show()
}
// Certificate is in any other way invalid
- it is QuasselSecurityException.Certificate -> {
+ it is QuasselSecurityException.Certificate -> {
MaterialDialog.Builder(this)
.title(R.string.label_error_certificate)
.content(
@@ -630,7 +630,7 @@ class ChatActivity : ServiceBoundActivity(), SharedPreferences.OnSharedPreferenc
.show()
}
// Certificate not valid for this hostname
- it is QuasselSecurityException.Hostname -> {
+ it is QuasselSecurityException.WrongHostname -> {
MaterialDialog.Builder(this)
.title(R.string.label_error_certificate)
.content(
diff --git a/lib/src/main/java/de/kuschku/libquassel/connection/CoreConnection.kt b/lib/src/main/java/de/kuschku/libquassel/connection/CoreConnection.kt
index f941732ef..26c84e6ed 100644
--- a/lib/src/main/java/de/kuschku/libquassel/connection/CoreConnection.kt
+++ b/lib/src/main/java/de/kuschku/libquassel/connection/CoreConnection.kt
@@ -29,7 +29,6 @@ import de.kuschku.libquassel.protocol.primitive.serializer.VariantListSerializer
import de.kuschku.libquassel.quassel.ProtocolFeature
import de.kuschku.libquassel.quassel.QuasselFeatures
import de.kuschku.libquassel.session.ProtocolHandler
-import de.kuschku.libquassel.ssl.BrowserCompatibleHostnameVerifier
import de.kuschku.libquassel.ssl.TrustManagers
import de.kuschku.libquassel.util.Optional
import de.kuschku.libquassel.util.compatibility.CompatibilityUtils
@@ -50,6 +49,8 @@ import java.lang.Thread.UncaughtExceptionHandler
import java.net.Socket
import java.net.SocketException
import java.nio.ByteBuffer
+import javax.net.ssl.HostnameVerifier
+import javax.net.ssl.HttpsURLConnection
import javax.net.ssl.SSLSession
import javax.net.ssl.X509TrustManager
@@ -60,7 +61,7 @@ class CoreConnection(
private val features: Features = Features(clientData.clientFeatures, QuasselFeatures.empty()),
private val handlerService: HandlerService = JavaHandlerService(),
private val trustManager: X509TrustManager = TrustManagers.default(),
- private val hostnameVerifier: HostnameVerifier = BrowserCompatibleHostnameVerifier()
+ private val hostnameVerifier: HostnameVerifier = HttpsURLConnection.getDefaultHostnameVerifier()
) : Thread(), Closeable {
companion object {
private const val TAG = "CoreConnection"
diff --git a/lib/src/main/java/de/kuschku/libquassel/connection/QuasselSecurityException.kt b/lib/src/main/java/de/kuschku/libquassel/connection/QuasselSecurityException.kt
index a08e28042..79aaeb8c1 100644
--- a/lib/src/main/java/de/kuschku/libquassel/connection/QuasselSecurityException.kt
+++ b/lib/src/main/java/de/kuschku/libquassel/connection/QuasselSecurityException.kt
@@ -31,11 +31,14 @@ sealed class QuasselSecurityException(
cause: Exception
) : QuasselSecurityException(certificateChain, cause)
- class Hostname(
+ class WrongHostname(
certificateChain: Array<out X509Certificate>?,
- val address: SocketAddress,
- cause: Exception
- ) : QuasselSecurityException(certificateChain, cause)
+ val address: SocketAddress
+ ) : QuasselSecurityException(certificateChain, null)
+
+ class NoCertificate(
+ val address: SocketAddress
+ ) : QuasselSecurityException(emptyArray(), null)
object NoSsl : QuasselSecurityException(emptyArray(), null)
}
diff --git a/lib/src/main/java/de/kuschku/libquassel/session/Session.kt b/lib/src/main/java/de/kuschku/libquassel/session/Session.kt
index d1549dd04..eb0ec7ada 100644
--- a/lib/src/main/java/de/kuschku/libquassel/session/Session.kt
+++ b/lib/src/main/java/de/kuschku/libquassel/session/Session.kt
@@ -26,7 +26,6 @@ import de.kuschku.libquassel.protocol.message.SignalProxyMessage
import de.kuschku.libquassel.quassel.ExtendedFeature
import de.kuschku.libquassel.quassel.QuasselFeatures
import de.kuschku.libquassel.quassel.syncables.*
-import de.kuschku.libquassel.ssl.BrowserCompatibleHostnameVerifier
import de.kuschku.libquassel.ssl.TrustManagers
import de.kuschku.libquassel.util.compatibility.HandlerService
import de.kuschku.libquassel.util.compatibility.LoggingHandler.Companion.log
@@ -37,6 +36,8 @@ import io.reactivex.Observable
import io.reactivex.subjects.BehaviorSubject
import io.reactivex.subjects.PublishSubject
import org.threeten.bp.Instant
+import javax.net.ssl.HostnameVerifier
+import javax.net.ssl.HttpsURLConnection
import javax.net.ssl.X509TrustManager
class Session(
@@ -44,7 +45,7 @@ class Session(
private var userData: Pair<String, String>,
requireSsl: Boolean = false,
trustManager: X509TrustManager = TrustManagers.default(),
- hostnameVerifier: HostnameVerifier = BrowserCompatibleHostnameVerifier(),
+ hostnameVerifier: HostnameVerifier = HttpsURLConnection.getDefaultHostnameVerifier(),
clientData: ClientData = ClientData.DEFAULT,
private val handlerService: HandlerService = JavaHandlerService(),
heartBeatFactory: () -> HeartBeatRunner = ::JavaHeartBeatRunner,
diff --git a/lib/src/main/java/de/kuschku/libquassel/session/manager/ConnectionInfo.kt b/lib/src/main/java/de/kuschku/libquassel/session/manager/ConnectionInfo.kt
index f93e7de18..da961d408 100644
--- a/lib/src/main/java/de/kuschku/libquassel/session/manager/ConnectionInfo.kt
+++ b/lib/src/main/java/de/kuschku/libquassel/session/manager/ConnectionInfo.kt
@@ -19,9 +19,9 @@
package de.kuschku.libquassel.session.manager
-import de.kuschku.libquassel.connection.HostnameVerifier
import de.kuschku.libquassel.connection.SocketAddress
import de.kuschku.libquassel.protocol.ClientData
+import javax.net.ssl.HostnameVerifier
import javax.net.ssl.X509TrustManager
data class ConnectionInfo(
diff --git a/lib/src/main/java/de/kuschku/libquassel/ssl/BrowserCompatibleHostnameVerifier.kt b/lib/src/main/java/de/kuschku/libquassel/ssl/BrowserCompatibleHostnameVerifier.kt
deleted file mode 100644
index d4aa2d6ac..000000000
--- a/lib/src/main/java/de/kuschku/libquassel/ssl/BrowserCompatibleHostnameVerifier.kt
+++ /dev/null
@@ -1,63 +0,0 @@
-/*
- * Quasseldroid - Quassel client for Android
- *
- * Copyright (c) 2020 Janne Mareike Koschinski
- * Copyright (c) 2020 The Quassel Project
- *
- * This program is free software: you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 3 as published
- * by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-
-package de.kuschku.libquassel.ssl
-
-import de.kuschku.libquassel.connection.HostnameVerifier
-import de.kuschku.libquassel.connection.SocketAddress
-import de.kuschku.libquassel.ssl.X509Helper.hostnames
-import java.net.IDN
-import java.security.cert.X509Certificate
-import javax.net.ssl.SSLException
-
-class BrowserCompatibleHostnameVerifier : HostnameVerifier {
- override fun checkValid(address: SocketAddress, chain: Array<out X509Certificate>) {
- val leafCertificate = chain.firstOrNull() ?: throw SSLException("No Certificate found")
- val hostnames = hostnames(leafCertificate).toList()
- if (hostnames.none { matches(it, address.host) })
- throw SSLException("Hostname does not match")
- }
-
- private fun matches(name: String, host: String): Boolean {
- // First we normalize both by removing trailing dots (absolute DNS names), splitting into DNS
- // labels, and punycoding all unicode parts.
- val normalizedName = name.trimEnd('.').split('.').map(IDN::toASCII)
- val normalizedHost = host.trimEnd('.').split('.').map(IDN::toASCII)
-
- // Only if both have the same number of DNS labels they can match
- if (normalizedHost.size != normalizedName.size) return false
-
- // Hosts with size of zero are invalid
- if (normalizedHost.isEmpty()) return false
-
- val both = normalizedName.zip(normalizedHost)
-
- // The first label has to either match exactly, or be *
- if (!both.take(1).all { (target, actual) ->
- target.equals(actual, ignoreCase = true) || target == "*"
- }) return false
-
- // All other labels have to match exactly.
- if (!both.drop(1).all { (target, actual) ->
- target.equals(actual, ignoreCase = true)
- }) return false
-
- return true
- }
-}
diff --git a/lib/src/main/java/de/kuschku/libquassel/ssl/TrustAllHostnameVerifier.kt b/lib/src/main/java/de/kuschku/libquassel/ssl/TrustAllHostnameVerifier.kt
deleted file mode 100644
index 75562629b..000000000
--- a/lib/src/main/java/de/kuschku/libquassel/ssl/TrustAllHostnameVerifier.kt
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * Quasseldroid - Quassel client for Android
- *
- * Copyright (c) 2020 Janne Mareike Koschinski
- * Copyright (c) 2020 The Quassel Project
- *
- * This program is free software: you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 3 as published
- * by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-
-package de.kuschku.libquassel.ssl
-
-import de.kuschku.libquassel.connection.HostnameVerifier
-import de.kuschku.libquassel.connection.SocketAddress
-import java.security.cert.X509Certificate
-
-class TrustAllHostnameVerifier : HostnameVerifier {
- override fun checkValid(address: SocketAddress, chain: Array<out X509Certificate>) = Unit
-}
diff --git a/lib/src/main/java/de/kuschku/libquassel/connection/HostnameVerifier.kt b/lib/src/main/java/de/kuschku/libquassel/ssl/X509CertificateHelper.kt
similarity index 74%
rename from lib/src/main/java/de/kuschku/libquassel/connection/HostnameVerifier.kt
rename to lib/src/main/java/de/kuschku/libquassel/ssl/X509CertificateHelper.kt
index 24c9e8d16..ca88664e4 100644
--- a/lib/src/main/java/de/kuschku/libquassel/connection/HostnameVerifier.kt
+++ b/lib/src/main/java/de/kuschku/libquassel/ssl/X509CertificateHelper.kt
@@ -17,12 +17,9 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-package de.kuschku.libquassel.connection
+package de.kuschku.libquassel.ssl
-import java.security.cert.X509Certificate
-import javax.net.ssl.SSLException
+import javax.security.cert.X509Certificate
-interface HostnameVerifier {
- @Throws(SSLException::class)
- fun checkValid(address: SocketAddress, chain: Array<out X509Certificate>)
-}
+fun X509Certificate.toJavaCertificate() = X509Helper.convert(this)
+fun Array<X509Certificate>.toJavaCertificate() = X509Helper.convert(this)
diff --git a/lib/src/main/java/de/kuschku/libquassel/ssl/X509Helper.kt b/lib/src/main/java/de/kuschku/libquassel/ssl/X509Helper.kt
index 42307e8bf..51412f517 100644
--- a/lib/src/main/java/de/kuschku/libquassel/ssl/X509Helper.kt
+++ b/lib/src/main/java/de/kuschku/libquassel/ssl/X509Helper.kt
@@ -23,9 +23,8 @@ import java.io.ByteArrayInputStream
import java.security.cert.CertificateFactory
import java.security.cert.X509Certificate
-// FIXME: re-read RFC and check it's actually secure
object X509Helper {
- val certificateFactory = CertificateFactory.getInstance("X.509")
+ private val certificateFactory = CertificateFactory.getInstance("X.509")
fun hostnames(certificate: X509Certificate): Sequence<String> =
(sequenceOf(certificate.subjectX500Principal.commonName) + subjectAlternativeNames(certificate))
@@ -48,6 +47,9 @@ object X509Helper {
fun convert(certificate: javax.security.cert.X509Certificate) =
certificateFactory.generateCertificate(ByteArrayInputStream(certificate.encoded)) as? X509Certificate
+ fun convert(chain: Array<out javax.security.cert.X509Certificate>): Array<X509Certificate>? =
+ chain.map { convert(it) ?: return null }.toTypedArray()
+
val COMMON_NAME = Regex("""(?:^|,\s?)(?:CN=("(?:[^"]|"")+"|[^,]+))""")
val ORGANIZATION = Regex("""(?:^|,\s?)(?:O=("(?:[^"]|"")+"|[^,]+))""")
diff --git a/lib/src/main/java/de/kuschku/libquassel/util/nio/WrappedChannel.kt b/lib/src/main/java/de/kuschku/libquassel/util/nio/WrappedChannel.kt
index 878ea7fc8..9b539999c 100644
--- a/lib/src/main/java/de/kuschku/libquassel/util/nio/WrappedChannel.kt
+++ b/lib/src/main/java/de/kuschku/libquassel/util/nio/WrappedChannel.kt
@@ -19,8 +19,9 @@
package de.kuschku.libquassel.util.nio
-import de.kuschku.libquassel.connection.HostnameVerifier
+import de.kuschku.libquassel.connection.QuasselSecurityException
import de.kuschku.libquassel.connection.SocketAddress
+import de.kuschku.libquassel.ssl.X509Helper
import de.kuschku.libquassel.util.compatibility.CompatibilityUtils
import de.kuschku.libquassel.util.compatibility.LoggingHandler.Companion.log
import de.kuschku.libquassel.util.compatibility.LoggingHandler.LogLevel
@@ -35,12 +36,8 @@ import java.nio.channels.ReadableByteChannel
import java.nio.channels.WritableByteChannel
import java.security.GeneralSecurityException
import java.security.NoSuchAlgorithmException
-import java.security.cert.X509Certificate
import java.util.zip.InflaterInputStream
-import javax.net.ssl.SSLContext
-import javax.net.ssl.SSLSocket
-import javax.net.ssl.SSLSocketFactory
-import javax.net.ssl.X509TrustManager
+import javax.net.ssl.*
class WrappedChannel private constructor(
private val socket: Socket,
@@ -91,8 +88,10 @@ class WrappedChannel private constructor(
}
@Throws(GeneralSecurityException::class, IOException::class)
- fun withSSL(certificateManager: X509TrustManager, hostnameVerifier: HostnameVerifier,
- address: SocketAddress): WrappedChannel {
+ fun withSSL(certificateManager: X509TrustManager,
+ hostnameVerifier: HostnameVerifier,
+ address: SocketAddress
+ ): WrappedChannel {
val tlsVersion = try {
selectBestTlsVersion(SSLContext.getDefault().defaultSSLParameters.protocols)
} catch (e: NoSuchAlgorithmException) {
@@ -111,10 +110,13 @@ class WrappedChannel private constructor(
val socket = factory.createSocket(socket, address.host, address.port, true) as SSLSocket
socket.useClientMode = true
socket.addHandshakeCompletedListener {
- hostnameVerifier.checkValid(
- address,
- socket.session.peerCertificates.map { it as X509Certificate }.toTypedArray()
- )
+ if (socket.session.peerCertificateChain.isEmpty()) {
+ throw QuasselSecurityException.NoCertificate(address)
+ }
+ if (!hostnameVerifier.verify(address.host, socket.session)) {
+ throw QuasselSecurityException.WrongHostname(X509Helper.convert(socket.session.peerCertificateChain),
+ address)
+ }
}
socket.startHandshake()
return ofSocket(socket)
--
GitLab