From 0a4725b962b1838f3cd064d25bc2bceb3e59d525 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 17 Nov 2014 22:21:18 +1100 Subject: [PATCH] Disable spongycastle before verifying jar files. Fixes issue #111. This is a workaround rather than a full solution, but it solves the problem. There is a bug in Lollipop, whereby including spongycastle as a security provider causes problems verifying .jar files. As a result, it is now disabled when actually performing verification, so that the libraries provided by Android do the work, and then re-enabled afterwards. There is an ever so slight chance that the period when this is disabled may align with the period when spongycastle may actually be required (i.e. for signing a local repo index). This is a risk which I cnanot see how to avoid, and will likel7 cause either the signing to fail due to the unavailability of the relevant security classes. However this is very minimal, hard to reproduce (I couldn't get it to happen) and also has the effect of the local repo failing, rather than the updating of apps failing (which is arguably more important) and so is worth it in my opinion. See comments at https://gitlab.com/fdroid/fdroidclient/issues/111 for much greater detail. --- src/org/fdroid/fdroid/FDroidApp.java | 27 ++++++++++-- src/org/fdroid/fdroid/data/App.java | 44 +++++++++++-------- .../fdroid/localrepo/LocalRepoKeyStore.java | 6 --- .../fdroid/updater/SignedRepoUpdater.java | 13 +++++- 4 files changed, 62 insertions(+), 28 deletions(-) diff --git a/src/org/fdroid/fdroid/FDroidApp.java b/src/org/fdroid/fdroid/FDroidApp.java index e16a5728c..c9ff2796f 100644 --- a/src/org/fdroid/fdroid/FDroidApp.java +++ b/src/org/fdroid/fdroid/FDroidApp.java @@ -55,10 +55,8 @@ import org.fdroid.fdroid.localrepo.LocalRepoService; import org.fdroid.fdroid.net.IconDownloader; import org.fdroid.fdroid.net.WifiStateChangeService; -import javax.net.ssl.HttpsURLConnection; -import javax.net.ssl.SSLContext; -import javax.net.ssl.TrustManager; import java.io.File; +import java.security.Security; import java.util.Set; public class FDroidApp extends Application { @@ -71,6 +69,8 @@ public class FDroidApp extends Application { public static Repo repo = new Repo(); public static Set selectedApps = null; // init in SelectLocalAppsFragment + // Leaving the fully qualified class name here to help clarify the difference between spongy/bouncy castle. + private static org.spongycastle.jce.provider.BouncyCastleProvider spongyCastleProvider; private static Messenger localRepoServiceMessenger = null; private static boolean localRepoServiceIsBound = false; @@ -78,6 +78,11 @@ public class FDroidApp extends Application { BluetoothAdapter bluetoothAdapter = null; + static { + spongyCastleProvider = new org.spongycastle.jce.provider.BouncyCastleProvider(); + enableSpongyCastle(); + } + public static enum Theme { dark, light, lightWithDarkActionBar } @@ -108,6 +113,22 @@ public class FDroidApp extends Application { return curTheme; } + public static void enableSpongyCastle() { + Security.addProvider(spongyCastleProvider); + } + + public static void enableSpongyCastleOnLollipop() { + if (Build.VERSION.SDK_INT == 21) { + Security.addProvider(spongyCastleProvider); + } + } + + public static void disableSpongyCastleOnLollipop() { + if (Build.VERSION.SDK_INT == 21) { + Security.removeProvider(spongyCastleProvider.getName()); + } + } + @Override public void onCreate() { super.onCreate(); diff --git a/src/org/fdroid/fdroid/data/App.java b/src/org/fdroid/fdroid/data/App.java index fd4af837c..933aa1ed0 100644 --- a/src/org/fdroid/fdroid/data/App.java +++ b/src/org/fdroid/fdroid/data/App.java @@ -11,6 +11,7 @@ import android.text.TextUtils; import android.util.Log; import org.fdroid.fdroid.AppFilter; +import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.Utils; import java.io.File; @@ -263,25 +264,32 @@ public class App extends ValueObject implements Comparable { throw new CertificateEncodingException("null signed entry!"); } - InputStream tmpIn = apkJar.getInputStream(aSignedEntry); - byte[] buff = new byte[2048]; - while (tmpIn.read(buff, 0, buff.length) != -1) { - /* - * NOP - apparently have to READ from the JarEntry before you can - * call getCerficates() and have it return != null. Yay Java. - */ + // Due to a bug in android 5.0 lollipop, the inclusion of BouncyCastle causes + // breakage when verifying the signature of most .jars. For more + // details, check out https://gitlab.com/fdroid/fdroidclient/issues/111. + try { + FDroidApp.disableSpongyCastleOnLollipop(); + InputStream tmpIn = apkJar.getInputStream(aSignedEntry); + byte[] buff = new byte[2048]; + while (tmpIn.read(buff, 0, buff.length) != -1) { + /* + * NOP - apparently have to READ from the JarEntry before you can + * call getCerficates() and have it return != null. Yay Java. + */ + } + tmpIn.close(); + + if (aSignedEntry.getCertificates() == null + || aSignedEntry.getCertificates().length == 0) { + apkJar.close(); + throw new CertificateEncodingException("No Certificates found!"); + } + + Certificate signer = aSignedEntry.getCertificates()[0]; + rawCertBytes = signer.getEncoded(); + } finally { + FDroidApp.enableSpongyCastleOnLollipop(); } - tmpIn.close(); - - if (aSignedEntry.getCertificates() == null - || aSignedEntry.getCertificates().length == 0) { - apkJar.close(); - throw new CertificateEncodingException("No Certificates found!"); - } - - Certificate signer = aSignedEntry.getCertificates()[0]; - rawCertBytes = signer.getEncoded(); - apkJar.close(); /* diff --git a/src/org/fdroid/fdroid/localrepo/LocalRepoKeyStore.java b/src/org/fdroid/fdroid/localrepo/LocalRepoKeyStore.java index b48a684e0..aafe89707 100644 --- a/src/org/fdroid/fdroid/localrepo/LocalRepoKeyStore.java +++ b/src/org/fdroid/fdroid/localrepo/LocalRepoKeyStore.java @@ -35,12 +35,6 @@ import kellinwood.security.zipsigner.ZipSigner; public class LocalRepoKeyStore { private static final String TAG = "KerplappKeyStore"; - - static { - Security.insertProviderAt( - new org.spongycastle.jce.provider.BouncyCastleProvider(), 1); - } - public static final String INDEX_CERT_ALIAS = "fdroid"; public static final String HTTP_CERT_ALIAS = "https"; diff --git a/src/org/fdroid/fdroid/updater/SignedRepoUpdater.java b/src/org/fdroid/fdroid/updater/SignedRepoUpdater.java index c6c9a4e48..31b14e7ec 100644 --- a/src/org/fdroid/fdroid/updater/SignedRepoUpdater.java +++ b/src/org/fdroid/fdroid/updater/SignedRepoUpdater.java @@ -2,6 +2,7 @@ package org.fdroid.fdroid.updater; import android.content.Context; import android.util.Log; +import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.Hasher; import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; @@ -123,7 +124,17 @@ public class SignedRepoUpdater extends RepoUpdater { // Don't worry about checking the status code for 200. If it was a // successful download, then we will have a file ready to use: if (indexJar != null && indexJar.exists()) { - indexXml = extractIndexFromJar(indexJar); + + // Due to a bug in android 5.0 lollipop, the inclusion of BouncyCastle causes + // breakage when verifying the signature of the downloaded .jar. For more + // details, check out https://gitlab.com/fdroid/fdroidclient/issues/111. + try { + FDroidApp.disableSpongyCastleOnLollipop(); + indexXml = extractIndexFromJar(indexJar); + } finally { + FDroidApp.enableSpongyCastleOnLollipop(); + } + } return indexXml; }