Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ dependencies {
testImplementation("com.google.crypto.tink:tink:1.23.0")
testImplementation("androidx.room:room-testing:$roomVersion")
testImplementation("com.squareup.okhttp3:mockwebserver:$okhttpVersion")
testImplementation("com.squareup.okhttp3:okhttp-tls:$okhttpVersion")
testImplementation("com.google.dagger:hilt-android-testing:2.60.1")
testImplementation("org.robolectric:robolectric:4.16.1")
// conscrypt-android provides Android JNI libs only; the openjdk-uber variant bundles
Expand Down
25 changes: 15 additions & 10 deletions app/src/main/java/com/nextcloud/talk/utils/ssl/TrustManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,19 +164,24 @@ private HostnameVerifier(javax.net.ssl.HostnameVerifier defaultHostNameVerifier)

@Override
public boolean verify(String s, SSLSession sslSession) {
try {
X509Certificate[] certificates = (X509Certificate[]) sslSession.getPeerCertificates();
if (certificates.length == 0) {
return false;
}

if (defaultHostNameVerifier.verify(s, sslSession)) {
try {
X509Certificate[] certificates = (X509Certificate[]) sslSession.getPeerCertificates();
if (certificates.length > 0 && isCertInTrustStore(certificates, s)) {
return true;
}
} catch (SSLPeerUnverifiedException e) {
Log.d(TAG, "Couldn't get certificate for host name verification");
if (defaultHostNameVerifier.verify(s, sslSession)) {
return true;
}
}

return false;
// Certificate has no (matching) Subject Alternative Name, e.g. legacy self-signed
// certificates that only set the Common Name. Accept it anyway if the user already
// explicitly trusted this exact certificate via the certificate dialog.
return isCertInTrustStore(certificates[0]);
} catch (SSLPeerUnverifiedException e) {
Log.d(TAG, "Couldn't get certificate for host name verification");
return false;
}
}
}

Expand Down
152 changes: 152 additions & 0 deletions app/src/test/java/com/nextcloud/talk/utils/ssl/TrustManagerTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/*
* Nextcloud Talk - Android Client
*
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: GPL-3.0-or-later
*/
package com.nextcloud.talk.utils.ssl

import android.app.Application
import android.content.Context
import androidx.test.core.app.ApplicationProvider
import com.nextcloud.talk.application.NextcloudTalkApplication
import okhttp3.tls.HeldCertificate
import org.junit.After
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.kotlin.mock
import org.mockito.kotlin.whenever
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
import org.robolectric.util.ReflectionHelpers
import java.security.cert.X509Certificate
import javax.net.ssl.HostnameVerifier
import javax.net.ssl.SSLPeerUnverifiedException
import javax.net.ssl.SSLSession

/**
* Regression tests for the hostname verifier returned by [TrustManager.getHostnameVerifier].
*
* Before the fix, a certificate without a matching Subject Alternative Name (e.g. legacy
* self-signed certificates that only set a Common Name) was always rejected, even after the
* user had explicitly trusted that exact certificate via the certificate dialog
* ([TrustManager.addCertInTrustStore]).
*/
@RunWith(RobolectricTestRunner::class)
@Config(application = Application::class, sdk = [33])
class TrustManagerTest {

private lateinit var trustManager: TrustManager

@Before
fun setUp() {
// TrustManager() needs NextcloudTalkApplication.sharedApplication for Context.getDir().
// Attach a real context without running NextcloudTalkApplication.onCreate(), which would
// pull in Dagger, WorkManager and WebRTC init that this test doesn't need.
val fakeApplication = NextcloudTalkApplication()
ReflectionHelpers.callInstanceMethod<Any?>(
fakeApplication,
"attachBaseContext",
ReflectionHelpers.ClassParameter.from(Context::class.java, ApplicationProvider.getApplicationContext())
)
setSharedApplication(fakeApplication)

trustManager = TrustManager()
}

@After
fun tearDown() {
setSharedApplication(null)
}

// NextcloudTalkApplication.sharedApplication's setter is `protected` (only meant to be set
// from onCreate()/onTerminate()); reflection is the only way in from a test.
private fun setSharedApplication(application: NextcloudTalkApplication?) {
ReflectionHelpers.callInstanceMethod<Any?>(
NextcloudTalkApplication.Companion,
"setSharedApplication",
ReflectionHelpers.ClassParameter.from(NextcloudTalkApplication::class.java, application)
)
}

@Test
fun `verify accepts connection when default hostname verifier passes`() {
// By the time the hostname verifier runs, checkServerTrusted() has already let the TLS
// handshake through for this exact certificate (CA-trusted, or previously trusted here).
val certificate = selfSignedCertificate(host = "example.com", withSan = true)
trustManager.addCertInTrustStore(certificate)

val sslSession = sessionWithCertificate(certificate)
val defaultVerifier = mock<HostnameVerifier>()
whenever(defaultVerifier.verify("example.com", sslSession)).thenReturn(true)

val hostnameVerifier = trustManager.getHostnameVerifier(defaultVerifier)

assertTrue(hostnameVerifier.verify("example.com", sslSession))
}

@Test
fun `verify rejects legacy certificate without SAN that was never manually trusted`() {
val certificate = selfSignedCertificate(host = "192.168.178.162", withSan = false)
val sslSession = sessionWithCertificate(certificate)
val defaultVerifier = mock<HostnameVerifier>()
whenever(defaultVerifier.verify("192.168.178.162", sslSession)).thenReturn(false)

val hostnameVerifier = trustManager.getHostnameVerifier(defaultVerifier)

assertFalse(hostnameVerifier.verify("192.168.178.162", sslSession))
}

@Test
fun `verify accepts legacy certificate without SAN once the user manually trusted it`() {
val certificate = selfSignedCertificate(host = "192.168.178.162", withSan = false)
trustManager.addCertInTrustStore(certificate)

val sslSession = sessionWithCertificate(certificate)
val defaultVerifier = mock<HostnameVerifier>()
whenever(defaultVerifier.verify("192.168.178.162", sslSession)).thenReturn(false)

val hostnameVerifier = trustManager.getHostnameVerifier(defaultVerifier)

assertTrue(hostnameVerifier.verify("192.168.178.162", sslSession))
}

@Test
fun `verify rejects connection when there are no peer certificates`() {
val sslSession = mock<SSLSession>()
whenever(sslSession.peerCertificates).thenReturn(arrayOf<X509Certificate>())
val defaultVerifier = mock<HostnameVerifier>()

val hostnameVerifier = trustManager.getHostnameVerifier(defaultVerifier)

assertFalse(hostnameVerifier.verify("example.com", sslSession))
}

@Test
fun `verify rejects connection when peer certificates cannot be read`() {
val sslSession = mock<SSLSession>()
whenever(sslSession.peerCertificates).thenThrow(SSLPeerUnverifiedException("no certificates"))
val defaultVerifier = mock<HostnameVerifier>()

val hostnameVerifier = trustManager.getHostnameVerifier(defaultVerifier)

assertFalse(hostnameVerifier.verify("example.com", sslSession))
}

private fun selfSignedCertificate(host: String, withSan: Boolean): X509Certificate {
val builder = HeldCertificate.Builder().commonName(host)
if (withSan) {
builder.addSubjectAlternativeName(host)
}
return builder.build().certificate
}

private fun sessionWithCertificate(certificate: X509Certificate): SSLSession {
val sslSession = mock<SSLSession>()
whenever(sslSession.peerCertificates).thenReturn(arrayOf(certificate))
return sslSession
}
}
Loading