From 3c6389c0047b98c06988e6023b579e0c44ac8e34 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 18 Jun 2015 10:25:07 -0400 Subject: [PATCH 1/8] fix failing symlink test the dest.txt symlink was produced, but it was pointing to a non-existent file. --- .../src/org/fdroid/fdroid/FileCompatTest.java | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/F-Droid/test/src/org/fdroid/fdroid/FileCompatTest.java b/F-Droid/test/src/org/fdroid/fdroid/FileCompatTest.java index 5ea33f3ee..22f1a2566 100644 --- a/F-Droid/test/src/org/fdroid/fdroid/FileCompatTest.java +++ b/F-Droid/test/src/org/fdroid/fdroid/FileCompatTest.java @@ -8,10 +8,11 @@ import org.fdroid.fdroid.compat.FileCompatForTest; import org.fdroid.fdroid.data.SanitizedFile; import java.io.File; +import java.util.UUID; public class FileCompatTest extends InstrumentationTestCase { - private static final String TAG = "org.fdroid.fdroid.FileCompatTest"; + private static final String TAG = "FileCompatTest"; private File dir; private SanitizedFile sourceFile; @@ -20,25 +21,22 @@ public class FileCompatTest extends InstrumentationTestCase { public void setUp() { dir = TestUtils.getWriteableDir(getInstrumentation()); sourceFile = SanitizedFile.knownSanitized(TestUtils.copyAssetToDir(getInstrumentation().getContext(), "simpleIndex.jar", dir)); - destFile = new SanitizedFile(dir, "dest.txt"); - assertTrue(!destFile.exists()); + destFile = new SanitizedFile(dir, "dest-" + UUID.randomUUID() + ".testproduct"); + assertFalse(destFile.exists()); assertTrue(sourceFile.getAbsolutePath() + " should exist.", sourceFile.exists()); } public void tearDown() { - if (sourceFile.exists()) { - assertTrue("Can't delete " + sourceFile.getAbsolutePath() + ".", sourceFile.delete()); + if (!sourceFile.delete()) { + System.out.println("Can't delete " + sourceFile.getAbsolutePath() + "."); } - if (destFile.exists()) { - assertTrue("Can't delete " + destFile.getAbsolutePath() + ".", destFile.delete()); + if (!destFile.delete()) { + System.out.println("Can't delete " + destFile.getAbsolutePath() + "."); } } public void testSymlinkRuntime() { - SanitizedFile destFile = new SanitizedFile(dir, "dest.txt"); - assertFalse(destFile.exists()); - FileCompatForTest.symlinkRuntimeTest(sourceFile, destFile); assertTrue(destFile.getAbsolutePath() + " should exist after symlinking", destFile.exists()); } @@ -46,9 +44,6 @@ public class FileCompatTest extends InstrumentationTestCase { public void testSymlinkLibcore() { if (Build.VERSION.SDK_INT >= 19) { - SanitizedFile destFile = new SanitizedFile(dir, "dest.txt"); - assertFalse(destFile.exists()); - FileCompatForTest.symlinkLibcoreTest(sourceFile, destFile); assertTrue(destFile.getAbsolutePath() + " should exist after symlinking", destFile.exists()); } else { @@ -59,9 +54,6 @@ public class FileCompatTest extends InstrumentationTestCase { public void testSymlinkOs() { if (Build.VERSION.SDK_INT >= 21 ) { - SanitizedFile destFile = new SanitizedFile(dir, "dest.txt"); - assertFalse(destFile.exists()); - FileCompatForTest.symlinkOsTest(sourceFile, destFile); assertTrue(destFile.getAbsolutePath() + " should exist after symlinking", destFile.exists()); } else { From d7efc99bdbc0bec9aecfb27f5905b4a1aa13d43e Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 15 Jun 2015 17:39:55 -0400 Subject: [PATCH 2/8] simplify RepoUpdater to remove cruft from previous code structure Before, there was an abstract RepoUpdater class with two subclasses, one for signed and unsigned. Now there is just a single class, and it only ever starts with the index.jar. So this removes lots of code that was there to handle that more complicated structure. For example, there is no longer the need to separately work on the index.xml vs index.jar. --- .../src/org/fdroid/fdroid/RepoUpdater.java | 89 +++++++------------ .../org/fdroid/fdroid/RepoUpdaterTest.java | 14 +-- 2 files changed, 40 insertions(+), 63 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java index 599ca7a41..ad0eda202 100644 --- a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java +++ b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java @@ -6,7 +6,6 @@ import android.content.pm.PackageManager; import android.os.Bundle; import android.support.annotation.NonNull; import android.support.annotation.Nullable; -import android.text.TextUtils; import android.util.Log; import org.fdroid.fdroid.data.Apk; @@ -68,39 +67,6 @@ public class RepoUpdater { public List getApks() { return apks; } - /** - * All repos are represented by a signed jar file, {@code index.jar}, which contains - * a single file, {@code index.xml}. This takes the {@code index.jar}, verifies the - * signature, then returns the unzipped {@code index.xml}. - * - * @throws UpdateException All error states will come from here. - */ - protected File getIndexFromFile(File downloadedFile) throws UpdateException { - final Date updateTime = new Date(System.currentTimeMillis()); - Log.d(TAG, "Getting signed index from " + repo.address + " at " + - Utils.formatLogDate(updateTime)); - - final File indexJar = downloadedFile; - File indexXml = null; - - // 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()) { - - // 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; - } - protected String getIndexAddress() { try { String versionName = context.getPackageManager().getPackageInfo(context.getPackageName(), 0).versionName; @@ -111,7 +77,7 @@ public class RepoUpdater { return repo.address + "/index.jar"; } - protected Downloader downloadIndex() throws UpdateException { + Downloader downloadIndex() throws UpdateException { Downloader downloader = null; try { downloader = DownloaderFactory.create( @@ -170,8 +136,17 @@ public class RepoUpdater { if (hasChanged) { + // 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: downloadedFile = downloader.getFile(); - indexFile = getIndexFromFile(downloadedFile); + + if (downloadedFile != null && downloadedFile.exists()) { + indexFile = getIndexFromJar(downloadedFile); + } + + if (indexXmlFile == null || !indexXmlFile.canRead()) { + throw new UpdateException(repo, "Failed to get index for " + repo.address); + } // Process the index... final SAXParser parser = SAXParserFactory.newInstance().newSAXParser(); @@ -200,7 +175,7 @@ public class RepoUpdater { } catch (SAXException | ParserConfigurationException | IOException e) { throw new UpdateException(repo, "Error parsing index for repo " + repo.address, e); } finally { - if (downloadedFile != null && downloadedFile != indexFile && downloadedFile.exists()) { + if (downloadedFile != null && downloadedFile.exists()) { downloadedFile.delete(); } if (indexFile != null && indexFile.exists()) { @@ -312,11 +287,27 @@ public class RepoUpdater { return match; } - protected File extractIndexFromJar(File indexJar) throws UpdateException { + /** + * All repos are represented by a signed jar file, {@code index.jar}, which contains + * a single file, {@code index.xml}. This takes the {@code index.jar}, verifies the + * signature, then returns the unzipped {@code index.xml}. + * + * @throws UpdateException All error states will come from here. + */ + protected File getIndexFromJar(File f) throws UpdateException { + final Date updateTime = new Date(System.currentTimeMillis()); + Log.d(TAG, "Getting signed index from " + repo.address + " at " + + Utils.formatLogDate(updateTime)); + File indexFile = null; JarFile jarFile = null; try { - jarFile = new JarFile(indexJar, true); + // 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. + FDroidApp.disableSpongyCastleOnLollipop(); + + jarFile = new JarFile(f, true); JarEntry indexEntry = (JarEntry) jarFile.getEntry("index.xml"); indexFile = File.createTempFile("index-", "-extracted.xml", context.getCacheDir()); @@ -340,11 +331,7 @@ public class RepoUpdater { // Can only read certificates from jar after it has been read // completely, so we put it after the copy above... - if (isTofuRequest()) { - Log.i(TAG, "Implicitly trusting the signature of index.jar, because this is a TOFU request"); - // Note that later on in the process we will save the pubkey against they repo, so - // that future requests verify against the signature we got this time. - } else if (!verifyCerts(indexEntry)) { + if (!verifyCerts(indexEntry)) { indexFile.delete(); throw new UpdateException(repo, "Index signature mismatch"); } @@ -352,9 +339,9 @@ public class RepoUpdater { if (indexFile != null) { indexFile.delete(); } - throw new UpdateException( - repo, "Error opening signed index", e); + throw new UpdateException(repo, "Error opening signed index", e); } finally { + FDroidApp.enableSpongyCastleOnLollipop(); if (jarFile != null) { try { jarFile.close(); @@ -367,14 +354,4 @@ public class RepoUpdater { return indexFile; } - /** - * If the repo doesn't have a fingerprint, then this is a "Trust On First Use" (TOFU) - * request. In that case, we will not verify the certificate, but rather implicitly trust - * the file we downloaded. We'll extract the certificate from the jar, and then use that - * to verify future requests to the same repository. - */ - private boolean isTofuRequest() { - return TextUtils.isEmpty(repo.fingerprint); - } - } diff --git a/F-Droid/test/src/org/fdroid/fdroid/RepoUpdaterTest.java b/F-Droid/test/src/org/fdroid/fdroid/RepoUpdaterTest.java index 96b2ae6f5..e441fcbd0 100644 --- a/F-Droid/test/src/org/fdroid/fdroid/RepoUpdaterTest.java +++ b/F-Droid/test/src/org/fdroid/fdroid/RepoUpdaterTest.java @@ -40,7 +40,7 @@ public class RepoUpdaterTest extends InstrumentationTestCase { // these are supposed to succeed try { - testFile = repoUpdater.getIndexFromFile(simpleIndexJar); + testFile = repoUpdater.getIndexFromJar(simpleIndexJar); assertTrue(testFile.length() == simpleIndexXml.length()); assertEquals(FileUtils.readFileToString(testFile), FileUtils.readFileToString(simpleIndexXml)); @@ -55,7 +55,7 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "simpleIndexWithoutSignature.jar", testFilesDir)); + repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithoutSignature.jar", testFilesDir)); fail(); } catch (UpdateException e) { // success! @@ -67,7 +67,7 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedManifest.jar", testFilesDir)); + repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedManifest.jar", testFilesDir)); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -82,7 +82,7 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedSignature.jar", testFilesDir)); + repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedSignature.jar", testFilesDir)); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -97,7 +97,7 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedCertificate.jar", testFilesDir)); + repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedCertificate.jar", testFilesDir)); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -112,7 +112,7 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedEverything.jar", testFilesDir)); + repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedEverything.jar", testFilesDir)); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -127,7 +127,7 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "masterKeyIndex.jar", testFilesDir)); + repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "masterKeyIndex.jar", testFilesDir)); fail(); } catch (UpdateException | SecurityException e) { // success! From 4f2650cd47a9df6913072507ed2371bf9fd73b5b Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 15 Jun 2015 17:40:28 -0400 Subject: [PATCH 3/8] update logic to match only parsing signed index files Now that there is only ever the index.jar, the whole flow of RepoUpdater has changed quite a bit. This updates the logic for deciding when to store the current repo's pubkey in the database for future reference. This changes the flow to stop writing the unpacked index.xml and instead stream it directly to the XML parser from the index.jar. This should speed things up some. refs #259 https://gitlab.com/fdroid/fdroidclient/issues/259 This is also work towards running the whole thing in the background: refs #103 https://gitlab.com/fdroid/fdroidclient/issues/103 This also removes the progress stuff since it will need to change a lot to work with the streaming mode --- .../src/org/fdroid/fdroid/RepoUpdater.java | 264 +++++++----------- .../src/org/fdroid/fdroid/RepoXMLHandler.java | 30 +- F-Droid/src/org/fdroid/fdroid/Utils.java | 31 -- .../org/fdroid/fdroid/RepoUpdaterTest.java | 30 +- 4 files changed, 119 insertions(+), 236 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java index ad0eda202..0b6e3a0d0 100644 --- a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java +++ b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java @@ -18,13 +18,11 @@ import org.xml.sax.InputSource; import org.xml.sax.SAXException; import org.xml.sax.XMLReader; -import java.io.BufferedReader; +import java.io.BufferedInputStream; import java.io.File; -import java.io.FileOutputStream; -import java.io.FileReader; import java.io.IOException; import java.io.InputStream; -import java.io.OutputStream; +import java.security.CodeSigner; import java.security.cert.Certificate; import java.util.ArrayList; import java.util.Date; @@ -48,13 +46,18 @@ public class RepoUpdater { private List apps = new ArrayList<>(); private List apks = new ArrayList<>(); private RepoUpdateRememberer rememberer = null; - protected boolean usePubkeyInJar = false; protected boolean hasChanged = false; @Nullable protected ProgressListener progressListener; - public RepoUpdater(@NonNull Context ctx, @NonNull Repo repo) { - this.context = ctx; - this.repo = repo; + /** + * Updates an app repo as read out of the database into a {@link Repo} instance. + * + * @param context + * @param repo a {@link Repo} read out of the local database + */ + public RepoUpdater(@NonNull Context context, @NonNull Repo repo) { + this.context = context; + this.repo = repo; } public void setProgressListener(ProgressListener progressListener) { @@ -107,86 +110,74 @@ public class RepoUpdater { return downloader; } - private int estimateAppCount(File indexFile) { - int count = -1; - try { - // A bit of a hack, this might return false positives if an apps description - // or some other part of the XML file contains this, but it is a pretty good - // estimate and makes the progress counter more informative. - // As with asking the server about the size of the index before downloading, - // this also has a time tradeoff. It takes about three seconds to iterate - // through the file and count 600 apps on a slow emulator (v17), but if it is - // taking two minutes to update, the three second wait may be worth it. - final String APPLICATION = " 1) { + throw new UpdateException(repo, "index.jar must be signed by a single code signer!"); + } + List certs = codeSigners[0].getSignerCertPath().getCertificates(); + if (certs.size() != 1) { + throw new UpdateException(repo, "index.jar code signers must only have a single certificate!"); + } + Certificate cert = certs.get(0); - Log.d(TAG, "Index has " + certs.length + " signature(s)"); - boolean match = false; - for (final Certificate cert : certs) { - String certdata = Hasher.hex(cert); - if (repo.pubkey == null && repo.fingerprint != null) { - String certFingerprint = Utils.calcFingerprint(cert); - Log.d(TAG, "No public key for repo " + repo.address + " yet, but it does have a fingerprint, so comparing them."); - Log.d(TAG, "Repo fingerprint: " + repo.fingerprint); - Log.d(TAG, "Cert fingerprint: " + certFingerprint); - if (repo.fingerprint.equalsIgnoreCase(certFingerprint)) { - repo.pubkey = certdata; - usePubkeyInJar = true; - } - } - if (repo.pubkey != null && repo.pubkey.equals(certdata)) { - Log.d(TAG, "Checking repo public key against cert found in jar."); - match = true; - break; + // though its called repo.pubkey, its actually a X509 certificate + String pubkey = Hasher.hex(cert); + + /* The first time a repo is added, it can be added with the signing key's + fingerprint. In that case, check that fingerprint against what is + actually in the index.jar itself */ + if (repo.pubkey == null && repo.fingerprint != null) { + String certFingerprint = Utils.calcFingerprint(cert); + if (repo.fingerprint.equalsIgnoreCase(certFingerprint)) { + storePubKey = true; + } else { + throw new UpdateException(repo, "Supplied certificate fingerprint does not match!"); } } - return match; - } - - /** - * All repos are represented by a signed jar file, {@code index.jar}, which contains - * a single file, {@code index.xml}. This takes the {@code index.jar}, verifies the - * signature, then returns the unzipped {@code index.xml}. - * - * @throws UpdateException All error states will come from here. - */ - protected File getIndexFromJar(File f) throws UpdateException { - final Date updateTime = new Date(System.currentTimeMillis()); - Log.d(TAG, "Getting signed index from " + repo.address + " at " + - Utils.formatLogDate(updateTime)); - - File indexFile = null; - JarFile jarFile = null; - try { - // 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. - FDroidApp.disableSpongyCastleOnLollipop(); - - jarFile = new JarFile(f, true); - JarEntry indexEntry = (JarEntry) jarFile.getEntry("index.xml"); - - indexFile = File.createTempFile("index-", "-extracted.xml", context.getCacheDir()); - InputStream input = null; - OutputStream output = null; - try { - /* - * JarFile.getInputStream() provides the signature check, even - * though the Android docs do not mention this, the Java docs do - * and Android seems to implement it the same: - * http://docs.oracle.com/javase/6/docs/api/java/util/jar/JarFile.html#getInputStream(java.util.zip.ZipEntry) - * https://developer.android.com/reference/java/util/jar/JarFile.html#getInputStream(java.util.zip.ZipEntry) - */ - input = jarFile.getInputStream(indexEntry); - output = new FileOutputStream(indexFile); - Utils.copy(input, output); - } finally { - Utils.closeQuietly(output); - Utils.closeQuietly(input); - } - - // Can only read certificates from jar after it has been read - // completely, so we put it after the copy above... - if (!verifyCerts(indexEntry)) { - indexFile.delete(); - throw new UpdateException(repo, "Index signature mismatch"); - } - } catch (IOException e) { - if (indexFile != null) { - indexFile.delete(); - } - throw new UpdateException(repo, "Error opening signed index", e); - } finally { - FDroidApp.enableSpongyCastleOnLollipop(); - if (jarFile != null) { - try { - jarFile.close(); - } catch (IOException ioe) { - // ignore - } - } + /* This storePubKey business makes me uncomfortable, but there seems no way around it + * since writing the pubkey to the database happens far from here in RepoUpdateRememberer */ + if (storePubKey) { + repo.pubkey = pubkey; + return; + } else if (repo.pubkey != null && repo.pubkey.equals(pubkey)) { + return; // we have a match! } - - return indexFile; + throw new UpdateException(repo, "Signing certificate does not match!"); } } diff --git a/F-Droid/src/org/fdroid/fdroid/RepoXMLHandler.java b/F-Droid/src/org/fdroid/fdroid/RepoXMLHandler.java index 8655f9edc..bf2853dc4 100644 --- a/F-Droid/src/org/fdroid/fdroid/RepoXMLHandler.java +++ b/F-Droid/src/org/fdroid/fdroid/RepoXMLHandler.java @@ -32,6 +32,9 @@ import org.xml.sax.helpers.DefaultHandler; import java.util.ArrayList; import java.util.List; +/** + * Parses the index.xml into Java data structures. + */ public class RepoXMLHandler extends DefaultHandler { // The repo we're processing. @@ -49,27 +52,18 @@ public class RepoXMLHandler extends DefaultHandler { private int version = -1; private int maxage = -1; - // After processing the XML, this will be null if the index specified a - // public key - otherwise a public key. This is used for TOFU where an - // index.xml is read on the first connection, and a signed index.jar is - // expected on all subsequent connections. + /** the pubkey stored in the header of index.xml */ private String pubkey; private String name; private String description; private String hashType; - private int progressCounter = 0; - private final ProgressListener progressListener; - - private int totalAppCount; - - public RepoXMLHandler(Repo repo, ProgressListener listener) { + public RepoXMLHandler(Repo repo) { this.repo = repo; pubkey = null; name = null; description = null; - progressListener = listener; } public List getApps() { return apps; } @@ -265,16 +259,6 @@ public class RepoXMLHandler extends DefaultHandler { } else if (localName.equals("application") && curapp == null) { curapp = new App(); curapp.id = attributes.getValue("", "id"); - /* show progress for the first 25, then start skipping every 25 */ - if (totalAppCount < 25 || progressCounter % (totalAppCount / 25) == 0) { - Bundle data = new Bundle(1); - data.putString(RepoUpdater.PROGRESS_DATA_REPO_ADDRESS, repo.address); - progressListener.onProgress( - new ProgressListener.Event( - RepoUpdater.PROGRESS_TYPE_PROCESS_XML, - progressCounter, totalAppCount, data)); - } - progressCounter++; } else if (localName.equals("package") && curapp != null && curapk == null) { curapk = new Apk(); curapk.id = curapp.id; @@ -287,10 +271,6 @@ public class RepoXMLHandler extends DefaultHandler { curchars.setLength(0); } - public void setTotalAppCount(int totalAppCount) { - this.totalAppCount = totalAppCount; - } - private String cleanWhiteSpace(String str) { return str.replaceAll("\n", " ").replaceAll(" ", " "); } diff --git a/F-Droid/src/org/fdroid/fdroid/Utils.java b/F-Droid/src/org/fdroid/fdroid/Utils.java index 42cd96ef3..cce9d61cd 100644 --- a/F-Droid/src/org/fdroid/fdroid/Utils.java +++ b/F-Droid/src/org/fdroid/fdroid/Utils.java @@ -45,7 +45,6 @@ import java.io.Closeable; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; -import java.io.FileReader; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -260,36 +259,6 @@ public final class Utils { return getMinMaxSdkVersion(context, packageName, "maxSdkVersion"); } - public static int countSubstringOccurrence(File file, String substring) throws IOException { - int count = 0; - FileReader input = null; - try { - int currentSubstringIndex = 0; - char[] buffer = new char[4096]; - - input = new FileReader(file); - int numRead = input.read(buffer); - while(numRead != -1) { - - for (char c : buffer) { - if (c == substring.charAt(currentSubstringIndex)) { - currentSubstringIndex++; - if (currentSubstringIndex == substring.length()) { - count++; - currentSubstringIndex = 0; - } - } else { - currentSubstringIndex = 0; - } - } - numRead = input.read(buffer); - } - } finally { - closeQuietly(input); - } - return count; - } - // return a fingerprint formatted for display public static String formatFingerprint(Context context, String fingerprint) { if (TextUtils.isEmpty(fingerprint) diff --git a/F-Droid/test/src/org/fdroid/fdroid/RepoUpdaterTest.java b/F-Droid/test/src/org/fdroid/fdroid/RepoUpdaterTest.java index e441fcbd0..1c5a95999 100644 --- a/F-Droid/test/src/org/fdroid/fdroid/RepoUpdaterTest.java +++ b/F-Droid/test/src/org/fdroid/fdroid/RepoUpdaterTest.java @@ -5,12 +5,11 @@ import android.annotation.TargetApi; import android.content.Context; import android.test.InstrumentationTestCase; -import org.apache.commons.io.FileUtils; import org.fdroid.fdroid.RepoUpdater.UpdateException; import org.fdroid.fdroid.data.Repo; import java.io.File; -import java.io.IOException; +import java.util.UUID; @TargetApi(8) public class RepoUpdaterTest extends InstrumentationTestCase { @@ -34,17 +33,12 @@ public class RepoUpdaterTest extends InstrumentationTestCase { public void testExtractIndexFromJar() { if (!testFilesDir.canWrite()) return; - File simpleIndexXml = TestUtils.copyAssetToDir(context, "simpleIndex.xml", testFilesDir); File simpleIndexJar = TestUtils.copyAssetToDir(context, "simpleIndex.jar", testFilesDir); - File testFile = null; // these are supposed to succeed try { - testFile = repoUpdater.getIndexFromJar(simpleIndexJar); - assertTrue(testFile.length() == simpleIndexXml.length()); - assertEquals(FileUtils.readFileToString(testFile), - FileUtils.readFileToString(simpleIndexXml)); - } catch (IOException | UpdateException e) { + repoUpdater.processDownloadedFile(simpleIndexJar, UUID.randomUUID().toString()); + } catch (UpdateException e) { e.printStackTrace(); fail(); } @@ -55,7 +49,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithoutSignature.jar", testFilesDir)); + File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithoutSignature.jar", testFilesDir); + repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString()); fail(); } catch (UpdateException e) { // success! @@ -67,7 +62,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedManifest.jar", testFilesDir)); + File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedManifest.jar", testFilesDir); + repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString()); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -82,7 +78,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedSignature.jar", testFilesDir)); + File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedSignature.jar", testFilesDir); + repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString()); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -97,7 +94,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedCertificate.jar", testFilesDir)); + File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedCertificate.jar", testFilesDir); + repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString()); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -112,7 +110,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedEverything.jar", testFilesDir)); + File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedEverything.jar", testFilesDir); + repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString()); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -127,7 +126,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "masterKeyIndex.jar", testFilesDir)); + File jarFile = TestUtils.copyAssetToDir(context, "masterKeyIndex.jar", testFilesDir); + repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString()); fail(); } catch (UpdateException | SecurityException e) { // success! From 64d709c142390fa2fe9c366df0a38df4c2ba5c14 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 18 Jun 2015 15:07:04 -0400 Subject: [PATCH 4/8] create ProgressBufferedInputStream to get progress info while parsing XML Might as well tap into the stream to get the byte counts, that's best progress info I can think of when parsing a file. This is a step towards a single progress bar for the whole process, instead of showing one progress for downloading, another for parsing XML, then a third for processing the new app info. --- F-Droid/res/values/strings.xml | 2 +- .../fdroid/ProgressBufferedInputStream.java | 51 +++++++++++++++++++ .../src/org/fdroid/fdroid/RepoUpdater.java | 14 ++--- .../src/org/fdroid/fdroid/UpdateService.java | 19 ++++--- 4 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 F-Droid/src/org/fdroid/fdroid/ProgressBufferedInputStream.java diff --git a/F-Droid/res/values/strings.xml b/F-Droid/res/values/strings.xml index bfc956e78..21fd192b0 100644 --- a/F-Droid/res/values/strings.xml +++ b/F-Droid/res/values/strings.xml @@ -211,7 +211,7 @@ - Percentage complete (int between 0-100) --> Downloading\n%2$s / %3$s (%4$d%%) from\n%1$s - Processing application\n%2$d of %3$d from\n%1$s + Processing\n%2$s / %3$s (%4$d%%) from\n%1$s Connecting to\n%1$s Checking apps compatibility with your device… Saving application details (%1$d%%) diff --git a/F-Droid/src/org/fdroid/fdroid/ProgressBufferedInputStream.java b/F-Droid/src/org/fdroid/fdroid/ProgressBufferedInputStream.java new file mode 100644 index 000000000..526d91105 --- /dev/null +++ b/F-Droid/src/org/fdroid/fdroid/ProgressBufferedInputStream.java @@ -0,0 +1,51 @@ +package org.fdroid.fdroid; + +import android.os.Bundle; + +import org.fdroid.fdroid.data.Repo; + +import java.io.BufferedInputStream; +import java.io.IOException; +import java.io.InputStream; + +public class ProgressBufferedInputStream extends BufferedInputStream { + private static final String TAG = "ProgressBufferedInputSt"; + + final Repo repo; + final ProgressListener progressListener; + final Bundle data; + final int totalBytes; + + int currentBytes = 0; + + /** + * Reports progress to the specified {@link ProgressListener}, with the + * progress based on the {@code totalBytes}. + */ + public ProgressBufferedInputStream(InputStream in, ProgressListener progressListener, Repo repo, int totalBytes) + throws IOException { + super(in); + this.progressListener = progressListener; + this.repo = repo; + this.data = new Bundle(1); + this.data.putString(RepoUpdater.PROGRESS_DATA_REPO_ADDRESS, repo.address); + this.totalBytes = totalBytes; + } + + @Override + public int read(byte[] buffer, int byteOffset, int byteCount) throws IOException { + if (progressListener != null) { + currentBytes += byteCount; + /* don't send every change to keep things efficient. 333333 bytes to keep all + * the digits changing because it looks pretty, < 9000 since the reads won't + * line up exactly */ + if (currentBytes % 333333 < 9000) { + progressListener.onProgress( + new ProgressListener.Event( + RepoUpdater.PROGRESS_TYPE_PROCESS_XML, + currentBytes, totalBytes, data)); + } + } + return super.read(buffer, byteOffset, byteCount); + } +} diff --git a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java index 0b6e3a0d0..f14a283fa 100644 --- a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java +++ b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java @@ -18,7 +18,6 @@ import org.xml.sax.InputSource; import org.xml.sax.SAXException; import org.xml.sax.XMLReader; -import java.io.BufferedInputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -136,12 +135,13 @@ public class RepoUpdater { if (repo.pubkey == null) // new repo, no signing certificate stored storePubKey = true; - JarEntry indexEntry = null; - if (downloadedFile != null && downloadedFile.exists()) { - JarFile jarFile = new JarFile(downloadedFile, true); - indexEntry = (JarEntry) jarFile.getEntry("index.xml"); - indexInputStream = new BufferedInputStream(jarFile.getInputStream(indexEntry)); - } + if (downloadedFile == null || !downloadedFile.exists()) + throw new UpdateException(repo, downloadedFile + " does not exist!"); + + JarFile jarFile = new JarFile(downloadedFile, true); + JarEntry indexEntry = (JarEntry) jarFile.getEntry("index.xml"); + indexInputStream = new ProgressBufferedInputStream(jarFile.getInputStream(indexEntry), + progressListener, repo, (int)indexEntry.getSize()); // Process the index... final SAXParser parser = SAXParserFactory.newInstance().newSAXParser(); diff --git a/F-Droid/src/org/fdroid/fdroid/UpdateService.java b/F-Droid/src/org/fdroid/fdroid/UpdateService.java index 43f3157bf..b4dc635d8 100644 --- a/F-Droid/src/org/fdroid/fdroid/UpdateService.java +++ b/F-Droid/src/org/fdroid/fdroid/UpdateService.java @@ -769,7 +769,6 @@ public class UpdateService extends IntentService implements ProgressListener { Log.d(TAG, "Removing " + numDeleted + " apks that don't have any apks"); } - /** * Received progress event from the RepoXMLHandler. It could be progress * downloading from the repo, or perhaps processing the info from the repo. @@ -780,16 +779,16 @@ public class UpdateService extends IntentService implements ProgressListener { // TODO: Switch to passing through Bundles of data with the event, rather than a repo address. They are // now much more general purpose then just repo downloading. String repoAddress = event.getData().getString(RepoUpdater.PROGRESS_DATA_REPO_ADDRESS); + String downloadedSize = Utils.getFriendlySize(event.progress); + String totalSize = Utils.getFriendlySize(event.total); + int percent = (int) ((double) event.progress / event.total * 100); switch (event.type) { - case Downloader.EVENT_PROGRESS: - String downloadedSize = Utils.getFriendlySize(event.progress); - String totalSize = Utils.getFriendlySize(event.total); - int percent = (int)((double)event.progress/event.total * 100); - message = getString(R.string.status_download, repoAddress, downloadedSize, totalSize, percent); - break; - case RepoUpdater.PROGRESS_TYPE_PROCESS_XML: - message = getString(R.string.status_processing_xml, repoAddress, event.progress, event.total); - break; + case Downloader.EVENT_PROGRESS: + message = getString(R.string.status_download, repoAddress, downloadedSize, totalSize, percent); + break; + case RepoUpdater.PROGRESS_TYPE_PROCESS_XML: + message = getString(R.string.status_processing_xml, repoAddress, downloadedSize, totalSize, percent); + break; } sendStatus(STATUS_INFO, message); } From 675151b4ef0b7443e4c36246326c5a6acf0a1b60 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 18 Jun 2015 19:44:43 -0400 Subject: [PATCH 5/8] rename string status_processing_xml to prevent crashes on old translations Thanks to @mvdan for catching that. Turns out Java's String formatting is not as tolerant as C's printf(). Java crashes when the format is wrong, while C just ignores extras. --- F-Droid/res/values-ca/strings.xml | 3 --- F-Droid/res/values-de/strings.xml | 3 --- F-Droid/res/values-el/strings.xml | 3 --- F-Droid/res/values-es/strings.xml | 3 --- F-Droid/res/values-fa/strings.xml | 3 --- F-Droid/res/values-fr/strings.xml | 3 --- F-Droid/res/values-gl/strings.xml | 3 --- F-Droid/res/values-hu/strings.xml | 3 --- F-Droid/res/values-it/strings.xml | 3 --- F-Droid/res/values-ja/strings.xml | 3 --- F-Droid/res/values-ko/strings.xml | 3 --- F-Droid/res/values-nb/strings.xml | 10 ++-------- F-Droid/res/values-nl/strings.xml | 3 --- F-Droid/res/values-pl/strings.xml | 1 - F-Droid/res/values-pt-rBR/strings.xml | 3 --- F-Droid/res/values-ru/strings.xml | 3 --- F-Droid/res/values-sr/strings.xml | 3 --- F-Droid/res/values-sv/strings.xml | 3 --- F-Droid/res/values-tr/strings.xml | 3 --- F-Droid/res/values-ug/strings.xml | 3 --- F-Droid/res/values-zh-rCN/strings.xml | 3 --- F-Droid/res/values/strings.xml | 2 +- F-Droid/src/org/fdroid/fdroid/UpdateService.java | 2 +- 23 files changed, 4 insertions(+), 68 deletions(-) diff --git a/F-Droid/res/values-ca/strings.xml b/F-Droid/res/values-ca/strings.xml index d909991ef..7851230ea 100644 --- a/F-Droid/res/values-ca/strings.xml +++ b/F-Droid/res/values-ca/strings.xml @@ -98,9 +98,6 @@ La voleu actualitzar? S\'ha actualitzat fa poc S\'està baixant %2$s / %3$s (%4$d%%) des de -%1$s - S\'està processant l\'aplicació -%2$d de %3$d des de %1$s S\'està connectant a %1$s diff --git a/F-Droid/res/values-de/strings.xml b/F-Droid/res/values-de/strings.xml index cfb2d0b9c..9d989fd67 100644 --- a/F-Droid/res/values-de/strings.xml +++ b/F-Droid/res/values-de/strings.xml @@ -171,9 +171,6 @@ Sollen diese aktualisiert werden? Konfigurieren Sie Ihren Proxy-Port (z.B. 8818) Herunterladen %2$s / %3$s (%4$d%%) von -%1$s - Anwendung wird vorbereitet -%2$d / %3$d von %1$s Mit %1$s wird verbunden diff --git a/F-Droid/res/values-el/strings.xml b/F-Droid/res/values-el/strings.xml index 13cbf27a0..1129a0120 100644 --- a/F-Droid/res/values-el/strings.xml +++ b/F-Droid/res/values-el/strings.xml @@ -123,9 +123,6 @@ Αναζήτηση για τοπικά αποθετήρια FDroid… Λήψη %2$s / %3$s (%4$d%%) από -%1$s - Επεξεργασία εφαρμογής -%2$d από %3$d από %1$s Σύνδεση με %1$s diff --git a/F-Droid/res/values-es/strings.xml b/F-Droid/res/values-es/strings.xml index 84650bdb7..b40c5be8f 100644 --- a/F-Droid/res/values-es/strings.xml +++ b/F-Droid/res/values-es/strings.xml @@ -170,9 +170,6 @@ La dirección de un repositorio es algo similar a esto: https://f-droid.org/repo Configurar el número del puerto del proxy (p.ej. 8118) Descargando %2$s / %3$s (%4$d%%) de -%1$s - Procesando la aplicación -%2$d de %3$d desde %1$s Conectando a %1$s diff --git a/F-Droid/res/values-fa/strings.xml b/F-Droid/res/values-fa/strings.xml index 0546cb911..4474ab384 100644 --- a/F-Droid/res/values-fa/strings.xml +++ b/F-Droid/res/values-fa/strings.xml @@ -82,9 +82,6 @@ همه دریافت %2$s / %3$s (%4$d%%) از -%1$s - پردازش برنامه -%2$d از %3$d از %1$s اتصال به %1$s diff --git a/F-Droid/res/values-fr/strings.xml b/F-Droid/res/values-fr/strings.xml index 40b29b615..ae83bc254 100644 --- a/F-Droid/res/values-fr/strings.xml +++ b/F-Droid/res/values-fr/strings.xml @@ -167,9 +167,6 @@ Voulez-vous les mettre à jour? Configurer votre numéro de port de proxy (ex. 8118) Téléchargement %2$s / %3$s (%4$d%%) de -%1$s - Prise en compte de l\'application -%2$d de %3$d depuis %1$s Connexion à %1$s diff --git a/F-Droid/res/values-gl/strings.xml b/F-Droid/res/values-gl/strings.xml index 0175f1829..c4e3032ea 100644 --- a/F-Droid/res/values-gl/strings.xml +++ b/F-Droid/res/values-gl/strings.xml @@ -95,9 +95,6 @@ Quere actualizalos? Actualizado recentemente Descargando %2$s / %3$s (%4$d%%) desde -%1$s - Procesando o aplicativo -%2$d de %3$d desde %1$s Conectándose con %1$s diff --git a/F-Droid/res/values-hu/strings.xml b/F-Droid/res/values-hu/strings.xml index 8b5f3c9d5..59f1f864b 100644 --- a/F-Droid/res/values-hu/strings.xml +++ b/F-Droid/res/values-hu/strings.xml @@ -113,9 +113,6 @@ Szeretné ezeket frissíteni?\" Legutóbb frissítve \"Letöltés %2$s / %3$s (%4$d%%) innen -%1$s\" - \"Feldolgozási kérelem -%2$d of %3$d innen %1$s\" \"Kapcsolódás %1$s\" diff --git a/F-Droid/res/values-it/strings.xml b/F-Droid/res/values-it/strings.xml index 758a24f8d..cb3aadad0 100644 --- a/F-Droid/res/values-it/strings.xml +++ b/F-Droid/res/values-it/strings.xml @@ -170,9 +170,6 @@ Vuoi aggiornarlo? Configura la porta del tuo proxy (es. 8118) Scaricamento %2$s / %3$s (%4$d%%) da -%1$s - Elaborazione applicazione -%2$d di %3$d da %1$s Connessione a %1$s diff --git a/F-Droid/res/values-ja/strings.xml b/F-Droid/res/values-ja/strings.xml index db1ea8799..c6e89b9da 100644 --- a/F-Droid/res/values-ja/strings.xml +++ b/F-Droid/res/values-ja/strings.xml @@ -172,9 +172,6 @@ GNU GPLv3 ライセンスに基づいてリリースされました. プロキシーのポート番号を設定(例:8118) ダウンロード中 %2$s / %3$s (%4$d%%) ダウンロード元 -%1$s - アプリケーションの処理中 -%2$d / %3$d %1$s 接続中 %1$s diff --git a/F-Droid/res/values-ko/strings.xml b/F-Droid/res/values-ko/strings.xml index 5baeb291d..8b3eba2d6 100644 --- a/F-Droid/res/values-ko/strings.xml +++ b/F-Droid/res/values-ko/strings.xml @@ -78,9 +78,6 @@ 최근 업데이트 %1$s 에서 다운로드 중입니다. %2$s / %3$s (%4$d%%) - 응용 프로그램 처리중 -%1$s -%2$d / %3$d %1$s에 접속중 장치와 응용프로그램의 호환성 확인중… 사용된 권한이 없습니다. diff --git a/F-Droid/res/values-nb/strings.xml b/F-Droid/res/values-nb/strings.xml index 6ac15c73d..1559ebb84 100644 --- a/F-Droid/res/values-nb/strings.xml +++ b/F-Droid/res/values-nb/strings.xml @@ -165,14 +165,8 @@ Lisensiert GNU GPLv3. Sett opp tjenernavn for din mellomtjener (f.eks. 127.0.0.1) Mellomtjener-port Sett opp portnummer for din mellomtjener (f.eks. 8118) - Laster ned -%2$s / %3$s (%4$d%%) fra -%1$s - Prosesserer applikasjon -%2$d of %3$d fra -%1$s - Kobler til -%1$s + Laster ned\n%2$s / %3$s (%4$d%%) fra\n%1$s + Kobler til\n%1$s Sjekker programstøtte for ditt utstyr… Lagrer programdata (%1$d%%) Ingen av pakkebrønnene hadde noen oppdateringer diff --git a/F-Droid/res/values-nl/strings.xml b/F-Droid/res/values-nl/strings.xml index 2be53666c..495cb9a79 100644 --- a/F-Droid/res/values-nl/strings.xml +++ b/F-Droid/res/values-nl/strings.xml @@ -124,9 +124,6 @@ Wilt u ze vernieuwen? Lokale FDroid opslagplaatsen Downloaden %2$s / %3$s (%4$d%%) van -%1$s - Verwerken applicatie -%2$d van %3$d van %1$s Verbinden met %1$s Controleer app compatibiliteit met uw apparaat… diff --git a/F-Droid/res/values-pl/strings.xml b/F-Droid/res/values-pl/strings.xml index fca30577e..eee271076 100644 --- a/F-Droid/res/values-pl/strings.xml +++ b/F-Droid/res/values-pl/strings.xml @@ -129,7 +129,6 @@ Czy chcesz je zaktualizować? Skonfiguruj host proxy Port proxy Skonfiguruj port proxy - Przetwarzanie aplikacji %2$d / %3$d z %1$s Trwa łączenie z %1$s Sprawdzanie kompatybilności aplikacji z urządzeniem… diff --git a/F-Droid/res/values-pt-rBR/strings.xml b/F-Droid/res/values-pt-rBR/strings.xml index 282342faf..9548ea21a 100644 --- a/F-Droid/res/values-pt-rBR/strings.xml +++ b/F-Droid/res/values-pt-rBR/strings.xml @@ -138,9 +138,6 @@ Você deseja atualizá-los? Configurar o número da porta do seu proxy (ex. 8118) Baixando %2$s / %3$s (%4$d%%) de -%1$s - Processando aplicativo -%2$d de %3$d, de %1$s Conectando-se a %1$s diff --git a/F-Droid/res/values-ru/strings.xml b/F-Droid/res/values-ru/strings.xml index 955d42a06..6487c539a 100644 --- a/F-Droid/res/values-ru/strings.xml +++ b/F-Droid/res/values-ru/strings.xml @@ -171,9 +171,6 @@ Настройка номера порта вашего прокси (напр. 8118) Загрузка %2$s / %3$s (%4$d%%) из -%1$s - Обработка приложения -%2$d из %3$d от %1$s Соединение с %1$s diff --git a/F-Droid/res/values-sr/strings.xml b/F-Droid/res/values-sr/strings.xml index 038a8d28d..c8b85e627 100644 --- a/F-Droid/res/values-sr/strings.xml +++ b/F-Droid/res/values-sr/strings.xml @@ -169,9 +169,6 @@ Подесите порт вашег проксија (нпр. 8118) Преузимам %2$s / %3$s (%4$d%%) са -%1$s - Обрађујем апликацију -%2$d од %3$d са %1$s Повезујем се са %1$s diff --git a/F-Droid/res/values-sv/strings.xml b/F-Droid/res/values-sv/strings.xml index 15d0ec554..9e71591b0 100644 --- a/F-Droid/res/values-sv/strings.xml +++ b/F-Droid/res/values-sv/strings.xml @@ -170,9 +170,6 @@ Vill du uppdatera dem? Konfigurera din proxys portnummer (t.ex. 8118) Hämtar %2$s / %3$s (%4$d%%) från -%1$s - Bearbetar program -%2$d av %3$d från %1$s Ansluter till %1$s diff --git a/F-Droid/res/values-tr/strings.xml b/F-Droid/res/values-tr/strings.xml index d79101f59..aebaa0d88 100644 --- a/F-Droid/res/values-tr/strings.xml +++ b/F-Droid/res/values-tr/strings.xml @@ -153,9 +153,6 @@ Güncellemek ister misiniz? Proxy port numarasını yapılandır (örn. 8118) İndiriliyor %2$s / %3$s (%4$d%%) şuradan -%1$s - Uygulama ele alınıyor -%2$d toplam %3$d şuradan %1$s %1$s konumuna bağlanılıyor diff --git a/F-Droid/res/values-ug/strings.xml b/F-Droid/res/values-ug/strings.xml index 0b1921925..0cf70f83d 100644 --- a/F-Droid/res/values-ug/strings.xml +++ b/F-Droid/res/values-ug/strings.xml @@ -91,9 +91,6 @@ يېقىنقى يېڭىلانغانلار چۈشۈرۈۋاتىدۇ %2$s / %3$s (%4$d%%) -%1$s - ئەپنى بىر تەرەپ قىلىۋاتىدۇ -%2$d of %3$d %1$s %1$s غا باغلىنىۋاتىدۇ diff --git a/F-Droid/res/values-zh-rCN/strings.xml b/F-Droid/res/values-zh-rCN/strings.xml index be9385e8a..d9cd34c5f 100644 --- a/F-Droid/res/values-zh-rCN/strings.xml +++ b/F-Droid/res/values-zh-rCN/strings.xml @@ -140,9 +140,6 @@ https://f-droid.org/repo 正在从以下位置下载: %1$s 进度:%2$s / %3$s (%4$d%%) - 正在处理以下位置的应用程序: -%1$s -进度:%2$d / %3$d 正在连接到 %1$s 正在检查应用程序与您的设备的兼容性… diff --git a/F-Droid/res/values/strings.xml b/F-Droid/res/values/strings.xml index 21fd192b0..f6e316951 100644 --- a/F-Droid/res/values/strings.xml +++ b/F-Droid/res/values/strings.xml @@ -211,7 +211,7 @@ - Percentage complete (int between 0-100) --> Downloading\n%2$s / %3$s (%4$d%%) from\n%1$s - Processing\n%2$s / %3$s (%4$d%%) from\n%1$s + Processing %2$s / %3$s (%4$d%%) from %1$s Connecting to\n%1$s Checking apps compatibility with your device… Saving application details (%1$d%%) diff --git a/F-Droid/src/org/fdroid/fdroid/UpdateService.java b/F-Droid/src/org/fdroid/fdroid/UpdateService.java index b4dc635d8..ec63a7d49 100644 --- a/F-Droid/src/org/fdroid/fdroid/UpdateService.java +++ b/F-Droid/src/org/fdroid/fdroid/UpdateService.java @@ -787,7 +787,7 @@ public class UpdateService extends IntentService implements ProgressListener { message = getString(R.string.status_download, repoAddress, downloadedSize, totalSize, percent); break; case RepoUpdater.PROGRESS_TYPE_PROCESS_XML: - message = getString(R.string.status_processing_xml, repoAddress, downloadedSize, totalSize, percent); + message = getString(R.string.status_processing_xml_percent, repoAddress, downloadedSize, totalSize, percent); break; } sendStatus(STATUS_INFO, message); From 43cc017c91822dbcb1fee279854d0bf6de94f565 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 22 Jun 2015 10:35:56 -0400 Subject: [PATCH 6/8] Utils.closeQuietly() for closing things in finally {} blocks --- F-Droid/src/org/fdroid/fdroid/RepoUpdater.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java index f14a283fa..0fe1e7528 100644 --- a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java +++ b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java @@ -164,13 +164,7 @@ public class RepoUpdater { } catch (SAXException | ParserConfigurationException | IOException e) { throw new UpdateException(repo, "Error parsing index for repo " + repo.address, e); } finally { - if (indexInputStream != null) { - try { - indexInputStream.close(); - } catch (IOException e) { - // ignored - } - } + Utils.closeQuietly(indexInputStream); if (downloadedFile != null) { downloadedFile.delete(); } From 2910acc9060c89c98af2ee7ae5c69258953fb469 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 22 Jun 2015 12:25:02 -0400 Subject: [PATCH 7/8] break out TOFU so all connections also run through regular verification This drastically simplifies the very important index.jar signature verification process by splitting out the Trust On First Use (TOFU) part of the process into its own method, and makes the TOFU write happen separately from the `RepoUpdateRememberer`. It requires all connections go through the normal verification process. --- .../src/org/fdroid/fdroid/RepoUpdater.java | 124 +++++++++++++----- .../src/org/fdroid/fdroid/RepoXMLHandler.java | 13 +- 2 files changed, 93 insertions(+), 44 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java index 0fe1e7528..04412da5e 100644 --- a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java +++ b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java @@ -6,6 +6,7 @@ import android.content.pm.PackageManager; import android.os.Bundle; import android.support.annotation.NonNull; import android.support.annotation.Nullable; +import android.text.TextUtils; import android.util.Log; import org.fdroid.fdroid.data.Apk; @@ -23,6 +24,7 @@ import java.io.IOException; import java.io.InputStream; import java.security.CodeSigner; import java.security.cert.Certificate; +import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -131,10 +133,6 @@ public class RepoUpdater { void processDownloadedFile(File downloadedFile, String cacheTag) throws UpdateException { InputStream indexInputStream = null; try { - boolean storePubKey = false; - if (repo.pubkey == null) // new repo, no signing certificate stored - storePubKey = true; - if (downloadedFile == null || !downloadedFile.exists()) throw new UpdateException(repo, downloadedFile + " does not exist!"); @@ -151,8 +149,16 @@ public class RepoUpdater { reader.parse(new InputSource(indexInputStream)); /* JarEntry can only read certificates after the file represented by that JarEntry - * has been read completely, so verification can run until now... */ - verifyCerts(indexEntry, storePubKey); + * has been read completely, so verification cannot run until now... */ + X509Certificate certFromJar = getSigningCertFromJar(indexEntry); + + String certFromIndexXml = repoXMLHandler.getSigningCertFromIndexXml(); + + // no signing cert read from database, this is the first use + if (repo.pubkey == null) { + verifyAndStoreTOFUCerts(certFromIndexXml, certFromJar); + } + verifyCerts(certFromIndexXml, certFromJar); apps = repoXMLHandler.getApps(); apks = repoXMLHandler.getApks(); @@ -160,7 +166,7 @@ public class RepoUpdater { rememberer = new RepoUpdateRememberer(); rememberer.context = context; rememberer.repo = repo; - rememberer.values = prepareRepoDetailsForSaving(repoXMLHandler, cacheTag, storePubKey); + rememberer.values = prepareRepoDetailsForSaving(repoXMLHandler, cacheTag); } catch (SAXException | ParserConfigurationException | IOException e) { throw new UpdateException(repo, "Error parsing index for repo " + repo.address, e); } finally { @@ -171,7 +177,11 @@ public class RepoUpdater { } } - private ContentValues prepareRepoDetailsForSaving(RepoXMLHandler handler, String etag, boolean storePubKey) { + /** + * Update tracking data for the repo represented by this instance (index version, etag, + * description, human-readable name, etc. + */ + private ContentValues prepareRepoDetailsForSaving(RepoXMLHandler handler, String etag) { ContentValues values = new ContentValues(); values.put(RepoProvider.DataColumns.LAST_UPDATED, Utils.formatDate(new Date(), "")); @@ -180,15 +190,6 @@ public class RepoUpdater { values.put(RepoProvider.DataColumns.LAST_ETAG, etag); } - /* - * We received a repo config that included the fingerprint, so we need to save - * the pubkey now. - */ - if (storePubKey && repo.pubkey != null && repo.pubkey.equals(handler.getPubKey())) { - Log.d(TAG, "Public key found - saving in the database."); - values.put(RepoProvider.DataColumns.PUBLIC_KEY, handler.getPubKey()); - } - if (handler.getVersion() != -1 && handler.getVersion() != repo.version) { Log.d(TAG, "Repo specified a new version: from " + repo.version + " to " + handler.getVersion()); @@ -196,8 +197,7 @@ public class RepoUpdater { } if (handler.getMaxAge() != -1 && handler.getMaxAge() != repo.maxage) { - Log.d(TAG, - "Repo specified a new maximum age - updated"); + Log.d(TAG, "Repo specified a new maximum age - updated"); values.put(RepoProvider.DataColumns.MAX_AGE, handler.getMaxAge()); } @@ -242,7 +242,12 @@ public class RepoUpdater { } } - private void verifyCerts(JarEntry jarEntry, boolean storePubKey) throws UpdateException { + /** + * FDroid's index.jar is signed using a particular format and does not allow lots of + * signing setups that would be valid for a regular jar. This validates those + * restrictions. + */ + private X509Certificate getSigningCertFromJar(JarEntry jarEntry) throws UpdateException { final CodeSigner[] codeSigners = jarEntry.getCodeSigners(); if (codeSigners == null || codeSigners.length == 0) { throw new UpdateException(repo, "No signature found in index"); @@ -255,28 +260,75 @@ public class RepoUpdater { if (certs.size() != 1) { throw new UpdateException(repo, "index.jar code signers must only have a single certificate!"); } - Certificate cert = certs.get(0); + return (X509Certificate) certs.get(0); + } - // though its called repo.pubkey, its actually a X509 certificate - String pubkey = Hasher.hex(cert); + /** + * A new repo can be added with or without the fingerprint of the signing + * certificate. If no fingerprint is supplied, then do a pure TOFU and just + * store the certificate as valid. If there is a fingerprint, then first + * check that the signing certificate in the jar matches that fingerprint. + */ + private void verifyAndStoreTOFUCerts(String certFromIndexXml, X509Certificate rawCertFromJar) + throws UpdateException { + if (repo.pubkey != null) + return; // there is a repo.pubkey already, nothing to TOFU - /* The first time a repo is added, it can be added with the signing key's - fingerprint. In that case, check that fingerprint against what is - actually in the index.jar itself */ - if (repo.pubkey == null && repo.fingerprint != null) { - String certFingerprint = Utils.calcFingerprint(cert); - if (repo.fingerprint.equalsIgnoreCase(certFingerprint)) { - storePubKey = true; + /* The first time a repo is added, it can be added with the signing certificate's + * fingerprint. In that case, check that fingerprint against what is + * actually in the index.jar itself. If no fingerprint, just store the + * signing certificate */ + boolean trustNewSigningCertificate = false; + if (repo.fingerprint == null) { + // no info to check things are valid, so just Trust On First Use + trustNewSigningCertificate = true; + } else { + String fingerprintFromIndexXml = Utils.calcFingerprint(certFromIndexXml); + String fingerprintFromJar = Utils.calcFingerprint(rawCertFromJar); + if (repo.fingerprint.equalsIgnoreCase(fingerprintFromIndexXml) + && repo.fingerprint.equalsIgnoreCase(fingerprintFromJar)) { + trustNewSigningCertificate = true; } else { throw new UpdateException(repo, "Supplied certificate fingerprint does not match!"); } } - /* This storePubKey business makes me uncomfortable, but there seems no way around it - * since writing the pubkey to the database happens far from here in RepoUpdateRememberer */ - if (storePubKey) { - repo.pubkey = pubkey; - return; - } else if (repo.pubkey != null && repo.pubkey.equals(pubkey)) { + + if (trustNewSigningCertificate) { + Log.d(TAG, "Saving new signing certificate in the database for " + repo.address); + ContentValues values = new ContentValues(2); + values.put(RepoProvider.DataColumns.LAST_UPDATED, Utils.formatDate(new Date(), "")); + values.put(RepoProvider.DataColumns.PUBLIC_KEY, Hasher.hex(rawCertFromJar)); + RepoProvider.Helper.update(context, repo, values); + } + } + + /** + * FDroid works with three copies of the signing certificate: + *
  • in the downloaded jar
  • + *
  • in the index XML
  • + *
  • stored in the local database
  • + * It would work better removing the copy from the index XML, but it needs to stay + * there for backwards compatibility since the old TOFU process requires it. Therefore, + * since all three have to be present, all three are compared. + * + * @param certFromIndexXml the cert written into the header of the index XML + * @param rawCertFromJar the {@link X509Certificate} embedded in the downloaded jar + */ + private void verifyCerts(String certFromIndexXml, X509Certificate rawCertFromJar) throws UpdateException { + // convert binary data to string version that is used in FDroid's database + String certFromJar = Hasher.hex(rawCertFromJar); + + // repo and repo.pubkey must be pre-loaded from the database + if (repo == null + || TextUtils.isEmpty(repo.pubkey) + || TextUtils.isEmpty(certFromJar) + || TextUtils.isEmpty(certFromIndexXml)) + throw new UpdateException(repo, "A empty repo or signing certificate is invalid!"); + + // though its called repo.pubkey, its actually a X509 certificate + if (repo.pubkey.equals(certFromJar) + && repo.pubkey.equals(certFromIndexXml) + && certFromIndexXml.equals(certFromJar)) { return; // we have a match! } throw new UpdateException(repo, "Signing certificate does not match!"); diff --git a/F-Droid/src/org/fdroid/fdroid/RepoXMLHandler.java b/F-Droid/src/org/fdroid/fdroid/RepoXMLHandler.java index bf2853dc4..e1c2c7642 100644 --- a/F-Droid/src/org/fdroid/fdroid/RepoXMLHandler.java +++ b/F-Droid/src/org/fdroid/fdroid/RepoXMLHandler.java @@ -52,8 +52,8 @@ public class RepoXMLHandler extends DefaultHandler { private int version = -1; private int maxage = -1; - /** the pubkey stored in the header of index.xml */ - private String pubkey; + /** the X.509 signing certificate stored in the header of index.xml */ + private String signingCertFromIndexXml; private String name; private String description; @@ -61,7 +61,7 @@ public class RepoXMLHandler extends DefaultHandler { public RepoXMLHandler(Repo repo) { this.repo = repo; - pubkey = null; + signingCertFromIndexXml = null; name = null; description = null; } @@ -78,7 +78,7 @@ public class RepoXMLHandler extends DefaultHandler { public String getName() { return name; } - public String getPubKey() { return pubkey; } + public String getSigningCertFromIndexXml() { return signingCertFromIndexXml; } @Override public void characters(char[] ch, int start, int length) { @@ -242,10 +242,7 @@ public class RepoXMLHandler extends DefaultHandler { super.startElement(uri, localName, qName, attributes); if (localName.equals("repo")) { - final String pk = attributes.getValue("", "pubkey"); - if (pk != null) - pubkey = pk; - + signingCertFromIndexXml = attributes.getValue("", "pubkey"); maxage = Utils.parseInt(attributes.getValue("", "maxage"), -1); version = Utils.parseInt(attributes.getValue("", "version"), -1); From 01fce7f94f6ac8518c0fffddec9b6c06fb406750 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 22 Jun 2015 12:57:23 -0400 Subject: [PATCH 8/8] re-enable Lollipop spongycastle hack It doesn't look like there is an easy way around this, so re-enable it in the new structure. #111 https://gitlab.com/fdroid/fdroidclient/issues/111 --- F-Droid/src/org/fdroid/fdroid/RepoUpdater.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java index 04412da5e..f3cff0b8b 100644 --- a/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java +++ b/F-Droid/src/org/fdroid/fdroid/RepoUpdater.java @@ -136,6 +136,11 @@ public class RepoUpdater { if (downloadedFile == null || !downloadedFile.exists()) throw new UpdateException(repo, downloadedFile + " does not exist!"); + // Due to a bug in Android 5.0 Lollipop, the inclusion of spongycastle causes + // breakage when verifying the signature of the downloaded .jar. For more + // details, check out https://gitlab.com/fdroid/fdroidclient/issues/111. + FDroidApp.disableSpongyCastleOnLollipop(); + JarFile jarFile = new JarFile(downloadedFile, true); JarEntry indexEntry = (JarEntry) jarFile.getEntry("index.xml"); indexInputStream = new ProgressBufferedInputStream(jarFile.getInputStream(indexEntry), @@ -170,6 +175,7 @@ public class RepoUpdater { } catch (SAXException | ParserConfigurationException | IOException e) { throw new UpdateException(repo, "Error parsing index for repo " + repo.address, e); } finally { + FDroidApp.enableSpongyCastleOnLollipop(); Utils.closeQuietly(indexInputStream); if (downloadedFile != null) { downloadedFile.delete();