From ac83a5a138b4a2b65e94b2d871e9b05d29a40aa1 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 1 Aug 2014 09:47:32 -0400 Subject: [PATCH 1/7] update to latest MemorizingTrustManager to get TrustManager bug fixes --- extern/MemorizingTrustManager | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extern/MemorizingTrustManager b/extern/MemorizingTrustManager index a705441ac..cd9bbf8f7 160000 --- a/extern/MemorizingTrustManager +++ b/extern/MemorizingTrustManager @@ -1 +1 @@ -Subproject commit a705441ac53b9e1aba9f00f3f59aab81da6fbc9e +Subproject commit cd9bbf8f7cc3cffa1abe1a7a2c775f345e7c489f From df3ba4c7513148b0e003807175ec8997a172394b Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 1 Aug 2014 09:50:29 -0400 Subject: [PATCH 2/7] only provide AndroidPinning TrustManager to MemorizingTrustManager The third parameter in the MemorizingTrustManager constructor was not good apparently. Here's the email from Ge0rg, the MemorizingTrustManager author: As you added MTM into the f-droid client, I'm writing to inform you that the MTM constructor API was incorrect, and has been changed in current git master: When using the three-parameter constructor, the second parameter, a trustmanager, was only used until the user stored a certificate into MTM, and was overwritten after that. Please use the new MTM constructor, and pass it the pinMgr as the only trust manager parameter. --- src/org/fdroid/fdroid/FDroidApp.java | 69 +++++++++++++++------------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/src/org/fdroid/fdroid/FDroidApp.java b/src/org/fdroid/fdroid/FDroidApp.java index 54111e958..ac6bd08bb 100644 --- a/src/org/fdroid/fdroid/FDroidApp.java +++ b/src/org/fdroid/fdroid/FDroidApp.java @@ -23,21 +23,34 @@ import android.app.Activity; import android.app.Application; import android.bluetooth.BluetoothAdapter; import android.bluetooth.BluetoothManager; -import android.content.*; -import android.content.pm.*; +import android.content.ComponentName; +import android.content.Context; +import android.content.Intent; +import android.content.ServiceConnection; +import android.content.SharedPreferences; +import android.content.pm.ApplicationInfo; +import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; +import android.content.pm.ResolveInfo; import android.net.Uri; import android.net.wifi.WifiManager; -import android.os.*; +import android.os.Build; +import android.os.IBinder; +import android.os.Message; +import android.os.Messenger; +import android.os.RemoteException; import android.preference.PreferenceManager; import android.util.Log; import android.widget.Toast; + import com.nostra13.universalimageloader.cache.disc.impl.LimitedAgeDiscCache; import com.nostra13.universalimageloader.cache.disc.naming.FileNameGenerator; import com.nostra13.universalimageloader.core.ImageLoader; import com.nostra13.universalimageloader.core.ImageLoaderConfiguration; import com.nostra13.universalimageloader.utils.StorageUtils; + import de.duenndns.ssl.MemorizingTrustManager; + import org.fdroid.fdroid.Preferences.ChangeListener; import org.fdroid.fdroid.compat.PRNGFixes; import org.fdroid.fdroid.data.AppProvider; @@ -49,16 +62,14 @@ import org.fdroid.fdroid.net.WifiStateChangeService; import org.thoughtcrime.ssl.pinning.PinningTrustManager; import org.thoughtcrime.ssl.pinning.SystemKeyStore; +import java.io.File; +import java.security.KeyManagementException; +import java.security.NoSuchAlgorithmException; +import java.util.Set; + import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLContext; import javax.net.ssl.TrustManager; -import javax.net.ssl.TrustManagerFactory; -import javax.net.ssl.X509TrustManager; -import java.io.File; -import java.security.*; -import java.util.Set; - -import javax.net.ssl.*; public class FDroidApp extends Application { @@ -179,31 +190,27 @@ public class FDroidApp extends Application { try { SSLContext sc = SSLContext.getInstance("TLS"); - X509TrustManager defaultTrustManager = null; + // MemorizingTrustManager -> PinningTrustManager -> Prompt User /* - * init a trust manager factory with a null keystore to access the system trust managers + * The current HTTPS trust model is to first check if a site's key + * is TOFUed, then check if it is pinned and valid with the CA, then + * prompt the user. There is currently no way to only check the CA + * for validity. Ultimately, that should probably not be needed if + * the repo URLs can include the HTTPS pin info in the same way that + * the repo fingerprint is specified. Then it can be added to the + * TOFU/POP keystore when the user accepts the Add Repo dialog */ - TrustManagerFactory tmf = - TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); - KeyStore ks = null; - tmf.init(ks); - TrustManager[] mgrs = tmf.getTrustManagers(); - - if(mgrs.length > 0 && mgrs[0] instanceof X509TrustManager) - defaultTrustManager = (X509TrustManager) mgrs[0]; + PinningTrustManager pinMgr = new PinningTrustManager( + SystemKeyStore.getInstance(getApplicationContext()), + FDroidCertPins.getPinList(), + 0); + MemorizingTrustManager memMgr = new MemorizingTrustManager(getApplicationContext(), pinMgr); /* - * compose a chain of trust managers as follows: - * MemorizingTrustManager -> Pinning Trust Manager -> System Trust Manager - */ - PinningTrustManager pinMgr = new PinningTrustManager(SystemKeyStore.getInstance(getApplicationContext()),FDroidCertPins.getPinList(), 0); - MemorizingTrustManager memMgr = new MemorizingTrustManager(getApplicationContext(), pinMgr, defaultTrustManager); - - /* - * initialize a SSLContext with the outermost trust manager, use this - * context to set the default SSL socket factory for the HTTPSURLConnection - * class. + * initialize a SSLContext with the outermost trust manager, use + * this context to set the default SSL socket factory for the + * HTTPSURLConnection class. */ sc.init(null, new TrustManager[] {memMgr}, new java.security.SecureRandom()); HttpsURLConnection.setDefaultSSLSocketFactory(sc.getSocketFactory()); @@ -211,8 +218,6 @@ public class FDroidApp extends Application { Log.e("FDroid", "Unable to set up trust manager chain. KeyManagementException"); } catch (NoSuchAlgorithmException e) { Log.e("FDroid", "Unable to set up trust manager chain. NoSuchAlgorithmException"); - } catch (KeyStoreException e) { - Log.e("FDroid", "Unable to set up trust manager chain. KeyStoreException"); } // initialized the local repo information From 9870fd13b63920e4e0cd2396253d10ac145a0d88 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 1 Aug 2014 23:13:39 -0400 Subject: [PATCH 3/7] ignore TrulyRandom lint warnings, fdroid already includes the fix PRNGFixes.apply() is run in FDroidApp.onCreate(). This is enough, according to Google, and their instructions say to disable this lint warning once the workaround is included since lint cannot detech whether the workaround is applied. This the code format was also automatically corrected by the Eclipse plugin --- lint.xml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lint.xml b/lint.xml index d8c98cba9..6e7f54486 100644 --- a/lint.xml +++ b/lint.xml @@ -1,11 +1,12 @@ - + - + + - + \ No newline at end of file From d2e32631d090cba13cab173752d4ac18143d3b4f Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 1 Aug 2014 23:24:52 -0400 Subject: [PATCH 4/7] fix incorrect lazy initialization of the list of HTTPS pins findbugs tells us: Incorrect lazy initialization and update of static field org.fdroid.fdroid. FDroidCertPins.PINLIST in org.fdroid.fdroid.FDroidCertPins.getPinList(). This method contains an unsynchronized lazy initialization of a static field. After the field is set, the object stored into that location is further updated or accessed. The setting of the field is visible to other threads as soon as it is set. If the futher accesses in the method that set the field serve to initialize the object, then you have a very serious multithreading bug, unless something else prevents any other thread from accessing the stored object until it is fully initialized. Even if you feel confident that the method is never called by multiple threads, it might be better to not set the static field until the value you are setting it to is fully populated/initialized. --- src/org/fdroid/fdroid/FDroidCertPins.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/org/fdroid/fdroid/FDroidCertPins.java b/src/org/fdroid/fdroid/FDroidCertPins.java index 646a792b2..cc7bb7f19 100644 --- a/src/org/fdroid/fdroid/FDroidCertPins.java +++ b/src/org/fdroid/fdroid/FDroidCertPins.java @@ -36,8 +36,9 @@ public class FDroidCertPins { public static String[] getPinList() { if (PINLIST == null) { - PINLIST = new ArrayList(); - PINLIST.addAll(Arrays.asList(DEFAULT_PINS)); + ArrayList pinlist = new ArrayList(); + pinlist.addAll(Arrays.asList(DEFAULT_PINS)); + PINLIST = pinlist; } return PINLIST.toArray(new String[PINLIST.size()]); From 249e38c32f534ee8c766551172597f18854790b5 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Sat, 2 Aug 2014 00:16:08 -0400 Subject: [PATCH 5/7] include relevant sites in list of HTTPS pins The current HTTPS trust model is to first check if a site's key is TOFUed, then check if it is pinned and check the CA, then prompt the user. There is currently no way to only check the CA for validity. Ultimately, that should probably not be needed if the repo URLs can include the HTTPS pin info in the same way that the repo fingerprint is specified. Then it can be added to the TOFU/POP keystore when the user accepts the Add Repo dialog Since that idea does not exist yet, this commit adds the sites that are likely to run their own repos in the near future: https://f-droid.org https://guardianproject.info https://s3.amazonaws.com # multiple orgs use this https://panicbutton.io # Amnesty International's app https://psiphon.ca # circumvention tool --- src/org/fdroid/fdroid/FDroidCertPins.java | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/org/fdroid/fdroid/FDroidCertPins.java b/src/org/fdroid/fdroid/FDroidCertPins.java index cc7bb7f19..d91563edb 100644 --- a/src/org/fdroid/fdroid/FDroidCertPins.java +++ b/src/org/fdroid/fdroid/FDroidCertPins.java @@ -23,13 +23,22 @@ import java.util.Arrays; public class FDroidCertPins { public static final String[] DEFAULT_PINS = { - /* - * SubjectDN: CN=f-droid.org, OU=PositiveSSL, OU=Domain Control Validated - * IssuerDN: CN=PositiveSSL CA 2, O=COMODO CA Limited, L=Salford, ST=Greater Manchester, C=GB - * Fingerprint: 84B91CDF2312CB9BA7F3BE803783302F8D8C299F - * SPKI Pin: 638F93856E1F5EDFCBD40C46D4160CFF21B0713A - */ - "638F93856E1F5EDFCBD40C46D4160CFF21B0713A", + + // OU=PositiveSSL, CN=f-droid.org + // Fingerprint: 84B91CDF2312CB9BA7F3BE803783302F8D8C299F + "638F93856E1F5EDFCBD40C46D4160CFF21B0713A", + + // OU=Gandi Standard SSL, CN=guardianproject.info + "cf2f8e226027599a1a933701418c58ec688a8305", + + // C=US, ST=Washington, L=Seattle, O=Amazon.com Inc., CN=s3.amazonaws.com + "5e77905babb66ca7082979435afbe4edf3f5af12", + + // OU=Domain Control Validated - RapidSSL(R), CN=www.psiphon.ca + "3aa1726e64d54bf58bf68fe23208928fd0d9cf8a", + + // OU=EssentialSSL Wildcard, CN=*.panicbutton.io + "cdae8cc70af09a55a7642d13f84241cba1c3a3e6", }; public static ArrayList PINLIST = null; From b695bbc4b1c6e46e4483b7a1fbd8d408ece99318 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Sat, 2 Aug 2014 00:27:47 -0400 Subject: [PATCH 6/7] fix crash after MemorizingTrustManager's TOFU/POP Always/Once/Abort prompt If there is an unknown HTTPS certificate, MemorizingTrustManager puts up a prompt to ask whether the user wants to trust the certificate. It comes at a weird time in the lifecycle of the dialogs, so the previous dialog might be null. Therefore add a null check. This situation should probably be improved and better integrated. --- src/org/fdroid/fdroid/UpdateService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/org/fdroid/fdroid/UpdateService.java b/src/org/fdroid/fdroid/UpdateService.java index 0638806c9..c7455047e 100644 --- a/src/org/fdroid/fdroid/UpdateService.java +++ b/src/org/fdroid/fdroid/UpdateService.java @@ -180,7 +180,7 @@ public class UpdateService extends IntentService implements ProgressListener { if (finished) { forwardEvent(EVENT_FINISHED); - if (dialog.isShowing()) { + if (dialog != null && dialog.isShowing()) { try { dialog.dismiss(); } catch (IllegalArgumentException e) { From 7b5e831b66d35375e39e903a9e0d18e46f1cb0a6 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 4 Aug 2014 18:53:26 -0400 Subject: [PATCH 7/7] darken category menu button on the dark theme to match the theme This commit uses alpha to make the category menu button appear darker to match the rest of the dark theme. Since the background is black, the alpha makes it darker. It is only used on the dark theme since alpha would lighten the menu button on the light themes, and that would make it a worse match. --- .../views/fragments/AvailableAppsFragment.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java b/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java index 6d43781b5..e8325ecd4 100644 --- a/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java +++ b/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java @@ -6,10 +6,13 @@ import android.content.SharedPreferences; import android.content.res.Resources; import android.database.ContentObserver; import android.database.Cursor; +import android.graphics.drawable.Drawable; import android.net.Uri; import android.os.Build; import android.os.Bundle; +import android.preference.PreferenceManager; import android.support.v4.app.LoaderManager; +import android.text.TextUtils; import android.util.Log; import android.view.LayoutInflater; import android.view.View; @@ -97,8 +100,7 @@ public class AvailableAppsFragment extends AppListFragment implements // attempt to translate category names with fallback to default name List translatedCategories = new ArrayList(categories.size()); Resources res = getResources(); - for (String category : categories) - { + for (String category : categories) { int id = res.getIdentifier(category.replace(" & ", "_"), "string", getActivity().getPackageName()); translatedCategories.add(id == 0 ? category : getString(id)); } @@ -108,8 +110,15 @@ public class AvailableAppsFragment extends AppListFragment implements // functionality do its stuff. categorySpinner.setId(R.id.categorySpinner); // with holo, the menu gets lost since it looks the same as an app list item - if (Build.VERSION.SDK_INT >= 14) - categorySpinner.setBackgroundDrawable(getResources().getDrawable(android.R.drawable.btn_dropdown)); + if (Build.VERSION.SDK_INT >= 14) { + Drawable menuButton = getResources().getDrawable(android.R.drawable.btn_dropdown); + if (TextUtils.equals("dark", + PreferenceManager.getDefaultSharedPreferences(getActivity()) + .getString(Preferences.PREF_THEME, "dark"))) { + menuButton.setAlpha(32); // make it darker via alpha + } + categorySpinner.setBackgroundDrawable(menuButton); + } ArrayAdapter adapter = new ArrayAdapter( getActivity(), android.R.layout.simple_spinner_item, translatedCategories);