From 5cbb4d5e583850521ba59261fec99878ec80d013 Mon Sep 17 00:00:00 2001 From: zerox80 Date: Thu, 2 Apr 2026 09:32:07 +0200 Subject: [PATCH 1/5] Enhance SSL handling and testing: Update SslUntrustedCertDialog for better certificate validation, remove hostname verifier in HttpClient, and add HttpClientTlsTest for SSL peer verification scenarios. --- .../ui/dialog/SslUntrustedCertDialog.java | 11 +- opencloudComLibrary/build.gradle | 1 + .../android/lib/common/http/HttpClient.java | 1 - .../operations/RemoteOperationResult.java | 5 + .../lib/common/http/HttpClientTlsTest.kt | 115 ++++++++++++++++++ 5 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 opencloudComLibrary/src/test/java/eu/opencloud/android/lib/common/http/HttpClientTlsTest.kt diff --git a/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java b/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java index dee1ea2e7..08ab1afa2 100644 --- a/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java +++ b/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java @@ -82,7 +82,9 @@ public static SslUntrustedCertDialog newInstanceForFullSslError(CertificateCombi throw new IllegalArgumentException("Trying to create instance with parameter sslException == null"); } SslUntrustedCertDialog dialog = new SslUntrustedCertDialog(); - dialog.m509Certificate = sslException.getServerCertificate(); + dialog.m509Certificate = sslException.getSslPeerUnverifiedException() == null + ? sslException.getServerCertificate() + : null; dialog.mErrorViewAdapter = new CertificateCombinedExceptionViewAdapter(sslException); dialog.mCertificateViewAdapter = new X509CertificateViewAdapter(sslException.getServerCertificate()); return dialog; @@ -141,6 +143,11 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle sa Button ok = mView.findViewById(R.id.ok); ok.setOnClickListener(new OnCertificateTrusted()); + if (m509Certificate == null) { + ok.setText(android.R.string.ok); + mView.findViewById(R.id.question).setVisibility(View.GONE); + } + Button cancel = mView.findViewById(R.id.btnCancel); cancel.setOnClickListener(new OnCertificateNotTrusted()); @@ -219,6 +226,8 @@ public void onClick(View v) { ((OnSslUntrustedCertListener) activity).onFailedSavingCertificate(); Timber.e(e, "Server certificate could not be saved in the known-servers trust store "); } + } else { + ((OnSslUntrustedCertListener) getActivity()).onCancelCertificate(); } } diff --git a/opencloudComLibrary/build.gradle b/opencloudComLibrary/build.gradle index d091ffbc9..bb66a5f96 100644 --- a/opencloudComLibrary/build.gradle +++ b/opencloudComLibrary/build.gradle @@ -27,6 +27,7 @@ dependencies { testImplementation 'org.robolectric:robolectric:4.15.1' // MockWebServer for HTTP integration tests testImplementation 'com.squareup.okhttp3:mockwebserver:4.9.2' + testImplementation 'com.squareup.okhttp3:okhttp-tls:4.9.2' // AndroidX test core to obtain application context in unit tests testImplementation 'androidx.test:core:1.5.0' debugImplementation 'com.facebook.stetho:stetho-okhttp3:1.6.0' diff --git a/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/http/HttpClient.java b/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/http/HttpClient.java index c70bb72c8..08ab5b8d3 100644 --- a/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/http/HttpClient.java +++ b/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/http/HttpClient.java @@ -125,7 +125,6 @@ private OkHttpClient buildNewOkHttpClient(SSLSocketFactory sslSocketFactory, X50 .connectTimeout(HttpConstants.DEFAULT_CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS) .followRedirects(false) .sslSocketFactory(sslSocketFactory, trustManager) - .hostnameVerifier((asdf, usdf) -> true) .cookieJar(cookieJar) .build(); } diff --git a/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/operations/RemoteOperationResult.java b/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/operations/RemoteOperationResult.java index 2fad3229c..16b786de4 100644 --- a/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/operations/RemoteOperationResult.java +++ b/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/operations/RemoteOperationResult.java @@ -148,6 +148,11 @@ public RemoteOperationResult(Exception e) { } else if (e instanceof SSLException || e instanceof RuntimeException) { if (e instanceof SSLPeerUnverifiedException) { + CertificateCombinedException sslPeerUnverifiedException = + new CertificateCombinedException(null); + sslPeerUnverifiedException.setSslPeerUnverifiedException((SSLPeerUnverifiedException) e); + sslPeerUnverifiedException.initCause(e); + mException = sslPeerUnverifiedException; mCode = ResultCode.SSL_RECOVERABLE_PEER_UNVERIFIED; } else { CertificateCombinedException se = getCertificateCombinedException(e); diff --git a/opencloudComLibrary/src/test/java/eu/opencloud/android/lib/common/http/HttpClientTlsTest.kt b/opencloudComLibrary/src/test/java/eu/opencloud/android/lib/common/http/HttpClientTlsTest.kt new file mode 100644 index 000000000..f677a267a --- /dev/null +++ b/opencloudComLibrary/src/test/java/eu/opencloud/android/lib/common/http/HttpClientTlsTest.kt @@ -0,0 +1,115 @@ +package eu.opencloud.android.lib.common.http + +import android.content.Context +import android.os.Build +import androidx.test.core.app.ApplicationProvider +import eu.opencloud.android.lib.common.network.CertificateCombinedException +import eu.opencloud.android.lib.common.network.NetworkUtils +import eu.opencloud.android.lib.common.operations.RemoteOperationResult +import okhttp3.Request +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import okhttp3.tls.HandshakeCertificates +import okhttp3.tls.HeldCertificate +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertThrows +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import java.lang.reflect.Field +import javax.net.ssl.SSLPeerUnverifiedException + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [Build.VERSION_CODES.O], manifest = Config.NONE) +class HttpClientTlsTest { + + private val context by lazy { ApplicationProvider.getApplicationContext() } + private lateinit var server: MockWebServer + + @Before + fun setUp() { + resetKnownServersStore() + server = MockWebServer() + } + + @After + fun tearDown() { + runCatching { server.shutdown() } + resetKnownServersStore() + } + + @Test + fun `rejects trusted certificate for the wrong hostname`() { + val wrongHostnameCertificate = HeldCertificate.Builder() + .commonName(WRONG_HOSTNAME) + .addSubjectAlternativeName(WRONG_HOSTNAME) + .build() + val serverCertificates = HandshakeCertificates.Builder() + .heldCertificate(wrongHostnameCertificate) + .build() + + server.useHttps(serverCertificates.sslSocketFactory(), false) + server.enqueue(MockResponse().setResponseCode(200).setBody("ok")) + server.start() + + NetworkUtils.addCertToKnownServersStore(wrongHostnameCertificate.certificate, context) + + val request = Request.Builder() + .url(server.url("/")) + .build() + + val thrown = assertThrows(Exception::class.java) { + TestHttpClient(context).okHttpClient.newCall(request).execute().use { } + } + + assertNotNull(findCause(thrown)) + } + + @Test + fun `wraps hostname mismatch as certificate combined exception`() { + val peerUnverifiedException = SSLPeerUnverifiedException("Hostname localhost not verified") + + val result = RemoteOperationResult(peerUnverifiedException) + + assertEquals( + RemoteOperationResult.ResultCode.SSL_RECOVERABLE_PEER_UNVERIFIED, + result.code + ) + assertTrue(result.exception is CertificateCombinedException) + + val combinedException = result.exception as CertificateCombinedException + assertSame(peerUnverifiedException, combinedException.sslPeerUnverifiedException) + } + + private inline fun findCause(throwable: Throwable): T? { + var current: Throwable? = throwable + while (current != null) { + if (current is T) { + return current + } + current = current.cause + } + return null + } + + private fun resetKnownServersStore() { + context.deleteFile(KNOWN_SERVERS_STORE_FILE) + + val field: Field = NetworkUtils::class.java.getDeclaredField("mKnownServersStore") + field.isAccessible = true + field.set(null, null) + } + + private class TestHttpClient(context: Context) : HttpClient(context) + + companion object { + private const val WRONG_HOSTNAME = "wrong.example.test" + private const val KNOWN_SERVERS_STORE_FILE = "knownServers.bks" + } +} \ No newline at end of file From 9c68f67fbd06bb08c4646904eb6d880bf2acdc34 Mon Sep 17 00:00:00 2001 From: zerox80 <115537871+zerox80@users.noreply.github.com> Date: Thu, 2 Apr 2026 10:15:28 +0200 Subject: [PATCH 2/5] Update opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../android/ui/dialog/SslUntrustedCertDialog.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java b/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java index 08ab1afa2..38b14b2a0 100644 --- a/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java +++ b/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java @@ -143,14 +143,14 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle sa Button ok = mView.findViewById(R.id.ok); ok.setOnClickListener(new OnCertificateTrusted()); + Button cancel = mView.findViewById(R.id.btnCancel); + cancel.setOnClickListener(new OnCertificateNotTrusted()); + if (m509Certificate == null) { ok.setText(android.R.string.ok); mView.findViewById(R.id.question).setVisibility(View.GONE); + cancel.setVisibility(View.GONE); } - - Button cancel = mView.findViewById(R.id.btnCancel); - cancel.setOnClickListener(new OnCertificateNotTrusted()); - Button details = mView.findViewById(R.id.details_btn); details.setOnClickListener(new OnClickListener() { From 7fdcac70c891c0b4115ba83815334b489b300613 Mon Sep 17 00:00:00 2001 From: zerox80 <115537871+zerox80@users.noreply.github.com> Date: Thu, 2 Apr 2026 10:16:38 +0200 Subject: [PATCH 3/5] Update opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java b/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java index 38b14b2a0..3632a648c 100644 --- a/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java +++ b/opencloudApp/src/main/java/eu/opencloud/android/ui/dialog/SslUntrustedCertDialog.java @@ -226,7 +226,7 @@ public void onClick(View v) { ((OnSslUntrustedCertListener) activity).onFailedSavingCertificate(); Timber.e(e, "Server certificate could not be saved in the known-servers trust store "); } - } else { + } else if (mHandler == null) { ((OnSslUntrustedCertListener) getActivity()).onCancelCertificate(); } } From 17ba765165a93b98e4a094a4e55ebe500ebd1369 Mon Sep 17 00:00:00 2001 From: zerox80 Date: Thu, 2 Apr 2026 10:45:28 +0200 Subject: [PATCH 4/5] Fix formatting: Add newline at end of HttpClientTlsTest.kt --- .../eu/opencloud/android/lib/common/http/HttpClientTlsTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opencloudComLibrary/src/test/java/eu/opencloud/android/lib/common/http/HttpClientTlsTest.kt b/opencloudComLibrary/src/test/java/eu/opencloud/android/lib/common/http/HttpClientTlsTest.kt index f677a267a..c58f3a8a5 100644 --- a/opencloudComLibrary/src/test/java/eu/opencloud/android/lib/common/http/HttpClientTlsTest.kt +++ b/opencloudComLibrary/src/test/java/eu/opencloud/android/lib/common/http/HttpClientTlsTest.kt @@ -112,4 +112,4 @@ class HttpClientTlsTest { private const val WRONG_HOSTNAME = "wrong.example.test" private const val KNOWN_SERVERS_STORE_FILE = "knownServers.bks" } -} \ No newline at end of file +} From 21f5f1d3df5df3b15cf0d07f323ece4c42c8fa3d Mon Sep 17 00:00:00 2001 From: zerox80 Date: Thu, 2 Apr 2026 18:47:43 +0200 Subject: [PATCH 5/5] Add ThreadLocal for last server certificate in AdvancedX509TrustManager and update RemoteOperationResult to utilize it for SSL exceptions --- .../android/lib/common/network/AdvancedX509TrustManager.java | 5 +++++ .../android/lib/common/operations/RemoteOperationResult.java | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/network/AdvancedX509TrustManager.java b/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/network/AdvancedX509TrustManager.java index e9b5b7754..1c1ff5b95 100644 --- a/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/network/AdvancedX509TrustManager.java +++ b/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/network/AdvancedX509TrustManager.java @@ -84,11 +84,16 @@ public void checkClientTrusted(X509Certificate[] certificates, String authType) mStandardTrustManager.checkClientTrusted(certificates, authType); } + public static final ThreadLocal sLastCert = new ThreadLocal<>(); + /** * @see javax.net.ssl.X509TrustManager#checkServerTrusted(X509Certificate[], * String authType) */ public void checkServerTrusted(X509Certificate[] certificates, String authType) { + if (certificates != null && certificates.length > 0) { + sLastCert.set(certificates[0]); + } if (!isKnownServer(certificates[0])) { CertificateCombinedException result = new CertificateCombinedException(certificates[0]); try { diff --git a/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/operations/RemoteOperationResult.java b/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/operations/RemoteOperationResult.java index 16b786de4..0cb82afa9 100644 --- a/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/operations/RemoteOperationResult.java +++ b/opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/operations/RemoteOperationResult.java @@ -32,6 +32,7 @@ import eu.opencloud.android.lib.common.accounts.AccountUtils; import eu.opencloud.android.lib.common.http.HttpConstants; import eu.opencloud.android.lib.common.http.methods.HttpBaseMethod; +import eu.opencloud.android.lib.common.network.AdvancedX509TrustManager; import eu.opencloud.android.lib.common.network.CertificateCombinedException; import okhttp3.Headers; import org.apache.commons.lang3.exception.ExceptionUtils; @@ -148,8 +149,10 @@ public RemoteOperationResult(Exception e) { } else if (e instanceof SSLException || e instanceof RuntimeException) { if (e instanceof SSLPeerUnverifiedException) { + java.security.cert.X509Certificate lastCert = AdvancedX509TrustManager.sLastCert.get(); + AdvancedX509TrustManager.sLastCert.remove(); CertificateCombinedException sslPeerUnverifiedException = - new CertificateCombinedException(null); + new CertificateCombinedException(lastCert); sslPeerUnverifiedException.setSslPeerUnverifiedException((SSLPeerUnverifiedException) e); sslPeerUnverifiedException.initCause(e); mException = sslPeerUnverifiedException;