From 6f5e6834f377cd6b4e271856690065304022fbef Mon Sep 17 00:00:00 2001 From: Mario Danic Date: Sun, 29 Oct 2017 23:08:36 +0100 Subject: [PATCH] Implement SSL error handling Signed-off-by: Mario Danic --- app/build.gradle | 2 + .../talk/activities/MainActivity.java | 82 ++++++++++++++++++- .../ServerSelectionController.java | 15 +++- .../controllers/WebViewLoginController.java | 53 +++++++++++- .../talk/dagger/modules/RestModule.java | 14 +++- .../talk/events/CertificateEvent.java | 43 ++++++++++ .../com/nextcloud/talk/utils/PushUtils.java | 3 - .../talk/utils/ssl/MagicTrustManager.java | 25 ++++-- .../res/drawable/ic_security_white_24dp.xml | 25 ++++++ app/src/main/res/values/colors.xml | 3 + app/src/main/res/values/strings.xml | 9 +- 11 files changed, 254 insertions(+), 20 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/events/CertificateEvent.java create mode 100644 app/src/main/res/drawable/ic_security_white_24dp.xml diff --git a/app/build.gradle b/app/build.gradle index 917b94153..11ed649b7 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -129,6 +129,8 @@ dependencies { implementation "com.google.android.gms:play-services-gcm:${googleLibraryVersion}" implementation "com.google.firebase:firebase-core:${googleLibraryVersion}" + implementation 'com.yarolegovich:lovely-dialog:1.0.7' + testImplementation 'junit:junit:4.12' androidTestImplementation ('com.android.support.test.espresso:espresso-core:3.0.1', { exclude group: 'com.android.support', module: 'support-annotations' diff --git a/app/src/main/java/com/nextcloud/talk/activities/MainActivity.java b/app/src/main/java/com/nextcloud/talk/activities/MainActivity.java index fb8282062..12dcb1811 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/MainActivity.java +++ b/app/src/main/java/com/nextcloud/talk/activities/MainActivity.java @@ -34,7 +34,19 @@ import com.nextcloud.talk.application.NextcloudTalkApplication; import com.nextcloud.talk.controllers.BottomNavigationController; import com.nextcloud.talk.controllers.ServerSelectionController; import com.nextcloud.talk.controllers.base.providers.ActionBarProvider; +import com.nextcloud.talk.events.CertificateEvent; import com.nextcloud.talk.utils.database.user.UserUtils; +import com.nextcloud.talk.utils.ssl.MagicTrustManager; +import com.yarolegovich.lovelydialog.LovelyStandardDialog; + +import org.greenrobot.eventbus.EventBus; +import org.greenrobot.eventbus.Subscribe; +import org.greenrobot.eventbus.ThreadMode; + +import java.security.cert.CertificateParsingException; +import java.security.cert.X509Certificate; +import java.text.DateFormat; +import java.util.List; import javax.inject.Inject; @@ -57,6 +69,9 @@ public final class MainActivity extends AppCompatActivity implements ActionBarPr @Inject ReactiveEntityStore dataStore; + @Inject + EventBus eventBus; + private Router router; @Override @@ -72,7 +87,6 @@ public final class MainActivity extends AppCompatActivity implements ActionBarPr router = Conductor.attachRouter(this, container, savedInstanceState); - if (!router.hasRootController() && userUtils.anyUserExists()) { router.setRoot(RouterTransaction.with(new BottomNavigationController(R.menu.menu_navigation)) .pushChangeHandler(new HorizontalChangeHandler()) @@ -90,4 +104,70 @@ public final class MainActivity extends AppCompatActivity implements ActionBarPr super.onBackPressed(); } } + + private void showCertificateDialog(X509Certificate cert, MagicTrustManager magicTrustManager) { + DateFormat formatter = DateFormat.getDateInstance(DateFormat.LONG); + String validFrom = formatter.format(cert.getNotBefore()); + String validUntil = formatter.format(cert.getNotAfter()); + + String issuedBy = cert.getIssuerDN().toString(); + String issuedFor; + + try { + if (cert.getSubjectAlternativeNames() != null) { + StringBuilder stringBuilder = new StringBuilder(); + for (Object o : cert.getSubjectAlternativeNames()) { + List list = (List) o; + int type = (Integer) list.get(0); + if (type == 2) { + String name = (String) list.get(1); + stringBuilder.append("[").append(type).append("]").append(name).append(" "); + } + } + issuedFor = stringBuilder.toString(); + } else { + issuedFor = cert.getSubjectDN().getName(); + } + + String dialogText = String.format(getResources() + .getString(R.string.nc_certificate_dialog_text), issuedBy, issuedFor, + validFrom, validUntil); + + new LovelyStandardDialog(this) + .setTopColorRes(R.color.darkRed) + .setNegativeButtonColorRes(R.color.darkRed) + .setPositiveButtonColorRes(R.color.colorPrimaryDark) + .setIcon(R.drawable.ic_security_white_24dp) + .setTitle(R.string.nc_certificate_dialog_title) + .setMessage(dialogText) + .setPositiveButton(R.string.nc_yes, v -> { + magicTrustManager.addCertInTrustStore(cert); + }) + .setNegativeButton(R.string.nc_no, view1 -> { + router.setRoot(RouterTransaction.with(new + ServerSelectionController())); + }) + .show(); + + } catch (CertificateParsingException e) { + e.printStackTrace(); + } + } + + @Subscribe(threadMode = ThreadMode.MAIN) + public void onMessageEvent(CertificateEvent event) { + showCertificateDialog(event.getX509Certificate(), event.getMagicTrustManager()); + }; + + @Override + public void onStart() { + super.onStart(); + eventBus.register(this); + } + + @Override + public void onStop() { + super.onStop(); + eventBus.unregister(this); + } } diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ServerSelectionController.java b/app/src/main/java/com/nextcloud/talk/controllers/ServerSelectionController.java index 89e44ccd0..d34bd8765 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ServerSelectionController.java +++ b/app/src/main/java/com/nextcloud/talk/controllers/ServerSelectionController.java @@ -36,6 +36,8 @@ import com.nextcloud.talk.api.helpers.api.ApiHelper; import com.nextcloud.talk.application.NextcloudTalkApplication; import com.nextcloud.talk.controllers.base.BaseController; +import java.security.cert.CertificateException; + import javax.inject.Inject; import autodagger.AutoInjector; @@ -123,10 +125,12 @@ public class ServerSelectionController extends BaseController { true); } else if (status.isNeedsUpgrade()) { textFieldBoxes.setError(String.format(getResources(). - getString(R.string.nc_server_db_upgrade_needed), productName), true); + getString(R.string.nc_server_db_upgrade_needed), + productName), true); } else if (status.isMaintenance()) { textFieldBoxes.setError(String.format(getResources(). - getString(R.string.nc_server_maintenance), productName), + getString(R.string.nc_server_maintenance), + productName), true); } else if (!status.getProductName().equals( getResources().getString(R.string.nc_server_product_name))) { @@ -137,7 +141,12 @@ public class ServerSelectionController extends BaseController { } }, throwable -> { - textFieldBoxes.setError(throwable.getLocalizedMessage(), true); + if (throwable.getLocalizedMessage() != null) { + textFieldBoxes.setError(throwable.getLocalizedMessage(), true); + } else if (throwable.getCause() instanceof CertificateException) { + textFieldBoxes.setError(getResources().getString(R.string.nc_certificate_error), + true); + } if (serverEntry != null) { serverEntry.setEnabled(true); } diff --git a/app/src/main/java/com/nextcloud/talk/controllers/WebViewLoginController.java b/app/src/main/java/com/nextcloud/talk/controllers/WebViewLoginController.java index fb5e5773e..12457c663 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/WebViewLoginController.java +++ b/app/src/main/java/com/nextcloud/talk/controllers/WebViewLoginController.java @@ -46,12 +46,15 @@ import com.nextcloud.talk.utils.bundle.BundleBuilder; import com.nextcloud.talk.utils.bundle.BundleKeys; import com.nextcloud.talk.utils.database.user.UserUtils; import com.nextcloud.talk.utils.ssl.MagicTrustManager; +import com.yarolegovich.lovelydialog.LovelyStandardDialog; import java.lang.reflect.Field; import java.net.URLDecoder; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.text.DateFormat; import java.util.HashMap; +import java.util.List; import java.util.Map; import javax.inject.Inject; @@ -148,7 +151,9 @@ public class WebViewLoginController extends BaseController { @Override public void onPageFinished(WebView view, String url) { if (!basePageLoaded) { - progressBar.setVisibility(View.GONE); + if (progressBar != null) { + progressBar.setVisibility(View.GONE); + } webView.setVisibility(View.VISIBLE); basePageLoaded = true; } @@ -171,8 +176,50 @@ public class WebViewLoginController extends BaseController { magicTrustManager.checkServerTrusted(new X509Certificate[]{cert}, "generic"); handler.proceed(); } catch (CertificateException exception) { - // cancel for now, as we don't have a way to accept custom certificates - handler.cancel(); + DateFormat formatter = DateFormat.getDateInstance(DateFormat.LONG); + String validFrom = formatter.format(cert.getNotBefore()); + String validUntil = formatter.format(cert.getNotAfter()); + + String issuedBy = cert.getIssuerDN().toString(); + String issuedFor; + + if (cert.getSubjectAlternativeNames() != null) { + StringBuilder stringBuilder = new StringBuilder(); + for (Object o : cert.getSubjectAlternativeNames()) { + List list = (List) o; + int type = (Integer) list.get(0); + if (type == 2) { + String name = (String) list.get(1); + stringBuilder.append("[").append(type).append("]").append(name).append(" "); + } + } + issuedFor = stringBuilder.toString(); + } else { + issuedFor = cert.getSubjectDN().getName(); + } + + String dialogText = String.format(getResources() + .getString(R.string.nc_certificate_dialog_text), issuedBy, issuedFor, + validFrom, validUntil); + + new LovelyStandardDialog(getActivity()) + .setTopColorRes(R.color.darkRed) + .setNegativeButtonColorRes(R.color.darkRed) + .setPositiveButtonColorRes(R.color.colorPrimaryDark) + .setIcon(R.drawable.ic_security_white_24dp) + .setTitle(R.string.nc_certificate_dialog_title) + .setMessage(dialogText) + .setPositiveButton(R.string.nc_yes, v -> { + magicTrustManager.addCertInTrustStore(cert); + handler.proceed(); + }) + .setNegativeButton(R.string.nc_no, view1 -> { + handler.cancel(); + getRouter().setRoot(RouterTransaction.with(new + ServerSelectionController())); + }) + .setCancelable(false) + .show(); } } } catch (Exception exception) { diff --git a/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java b/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java index 4bb4da3cc..e6faf2b30 100644 --- a/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java +++ b/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java @@ -37,6 +37,7 @@ import com.nextcloud.talk.utils.ssl.SSLSocketFactoryCompat; import java.io.IOException; import java.net.InetSocketAddress; import java.net.Proxy; +import java.util.concurrent.TimeUnit; import javax.inject.Singleton; @@ -59,6 +60,8 @@ import retrofit2.adapter.rxjava2.RxJava2CallAdapterFactory; @Module(includes = DatabaseModule.class) public class RestModule { + private static final String TAG = "RestModule"; + @Provides @Singleton NcApi provideNcApi(Retrofit retrofit) { @@ -98,7 +101,6 @@ public class RestModule { @Singleton MagicTrustManager provideMagicTrustManager() { return new MagicTrustManager(); - } @Provides @@ -110,9 +112,14 @@ public class RestModule { @Provides @Singleton OkHttpClient provideHttpClient(Proxy proxy, AppPreferences appPreferences, - MagicTrustManager magicTrustManager, SSLSocketFactoryCompat sslSocketFactoryCompat) { + MagicTrustManager magicTrustManager, + SSLSocketFactoryCompat sslSocketFactoryCompat) { OkHttpClient.Builder httpClient = new OkHttpClient.Builder(); + httpClient.connectTimeout(30, TimeUnit.SECONDS); + httpClient.readTimeout(30, TimeUnit.SECONDS); + httpClient.writeTimeout(30, TimeUnit.SECONDS); + int cacheSize = 128 * 1024 * 1024; // 128 MB httpClient.cache(new Cache(NextcloudTalkApplication.getSharedApplication().getCacheDir(), cacheSize)); @@ -125,7 +132,8 @@ public class RestModule { } httpClient.sslSocketFactory(sslSocketFactoryCompat, magicTrustManager); - httpClient.hostnameVerifier(magicTrustManager.getHostnameVerifier(OkHostnameVerifier.INSTANCE)); + httpClient.retryOnConnectionFailure(true); + httpClient.hostnameVerifier(new MagicTrustManager().getHostnameVerifier(OkHostnameVerifier.INSTANCE)); if (!Proxy.NO_PROXY.equals(proxy)) { httpClient.proxy(proxy); diff --git a/app/src/main/java/com/nextcloud/talk/events/CertificateEvent.java b/app/src/main/java/com/nextcloud/talk/events/CertificateEvent.java new file mode 100644 index 000000000..001a86dbe --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/events/CertificateEvent.java @@ -0,0 +1,43 @@ +/* + * Nextcloud Talk application + * + * @author Mario Danic + * Copyright (C) 2017 Mario Danic + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * at your option) any later version. + * + * 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 . + */ + +package com.nextcloud.talk.events; + +import com.nextcloud.talk.utils.ssl.MagicTrustManager; + +import java.security.cert.X509Certificate; + +public class CertificateEvent { + private final X509Certificate x509Certificate; + private final MagicTrustManager magicTrustManager; + + public CertificateEvent(X509Certificate x509Certificate, MagicTrustManager magicTrustManager) { + this.x509Certificate = x509Certificate; + this.magicTrustManager = magicTrustManager; + } + + public X509Certificate getX509Certificate() { + return x509Certificate; + } + + public MagicTrustManager getMagicTrustManager() { + return magicTrustManager; + } +} diff --git a/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java b/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java index 3b5704b06..cd5bb2115 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java +++ b/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java @@ -36,8 +36,6 @@ import com.nextcloud.talk.persistence.entities.UserEntity; import com.nextcloud.talk.utils.database.user.UserUtils; import com.nextcloud.talk.utils.preferences.AppPreferences; -import org.json.JSONObject; - import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -103,7 +101,6 @@ public class PushUtils { Signature signature = null; PushConfigurationState pushConfigurationState; PublicKey publicKey; - JSONObject jsonObject; SignatureVerification signatureVerification = new SignatureVerification(); signatureVerification.setSignatureValid(false); diff --git a/app/src/main/java/com/nextcloud/talk/utils/ssl/MagicTrustManager.java b/app/src/main/java/com/nextcloud/talk/utils/ssl/MagicTrustManager.java index 225de2bcb..f2e0209b7 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/ssl/MagicTrustManager.java +++ b/app/src/main/java/com/nextcloud/talk/utils/ssl/MagicTrustManager.java @@ -26,6 +26,9 @@ import android.content.Context; import android.util.Log; import com.nextcloud.talk.application.NextcloudTalkApplication; +import com.nextcloud.talk.events.CertificateEvent; + +import org.greenrobot.eventbus.EventBus; import java.io.File; import java.io.FileInputStream; @@ -50,10 +53,6 @@ public class MagicTrustManager implements X509TrustManager { private X509TrustManager systemTrustManager = null; private KeyStore trustedKeyStore = null; - public HostnameVerifier getHostnameVerifier(HostnameVerifier defaultHostNameVerifier) { - return new MagicHostnameVerifier(defaultHostNameVerifier); - } - public MagicTrustManager() { keystoreFile = new File(NextcloudTalkApplication.getSharedApplication().getDir("CertsKeystore", Context.MODE_PRIVATE), "keystore.bks"); @@ -89,15 +88,30 @@ public class MagicTrustManager implements X509TrustManager { } + public HostnameVerifier getHostnameVerifier(HostnameVerifier defaultHostNameVerifier) { + return new MagicHostnameVerifier(defaultHostNameVerifier); + } + public boolean isCertInTrustStore(X509Certificate x509Certificate) { if (systemTrustManager != null) { try { systemTrustManager.checkServerTrusted(new X509Certificate[]{x509Certificate}, "generic"); return true; } catch (CertificateException e) { - return isCertInMagicTrustStore(x509Certificate); + if (!isCertInMagicTrustStore(x509Certificate)) { + EventBus.getDefault().post(new CertificateEvent(x509Certificate, this)); + long startTime = System.currentTimeMillis(); + while (!isCertInMagicTrustStore(x509Certificate) && System.currentTimeMillis() <= + startTime + 15000) { + //do nothing + } + return isCertInMagicTrustStore(x509Certificate); + } else { + return true; + } } } + return false; } @@ -159,7 +173,6 @@ public class MagicTrustManager implements X509TrustManager { return true; } - try { X509Certificate[] certificates = (X509Certificate[]) sslSession.getPeerCertificates(); if (certificates.length > 0 && certificates[0] != null && isCertInMagicTrustStore(certificates[0])) { diff --git a/app/src/main/res/drawable/ic_security_white_24dp.xml b/app/src/main/res/drawable/ic_security_white_24dp.xml new file mode 100644 index 000000000..84c74c366 --- /dev/null +++ b/app/src/main/res/drawable/ic_security_white_24dp.xml @@ -0,0 +1,25 @@ + + + + + diff --git a/app/src/main/res/values/colors.xml b/app/src/main/res/values/colors.xml index 852f0bebb..7a89774e2 100644 --- a/app/src/main/res/values/colors.xml +++ b/app/src/main/res/values/colors.xml @@ -3,4 +3,7 @@ #0082c9 #006AA3 #007CC2 + + #D32F2F + diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 3ff088d53..dbef05568 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -22,7 +22,14 @@ Invited Never - Search + Trust the certificate? + We encountered an SSL certificate issued by %1$s for %2$s and valid + from %3$s to %4$s. Trust it? + Yes + No + Details + Your SSL setup prevented us from connecting +