Karl/BobStore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

simplify RepoUpdater to remove cruft from previous code structure

Browse Source 
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.
This commit is contained in:
Hans-Christoph Steiner 2015-06-15 17:39:55 -04:00
parent 3c6389c004
commit d7efc99bdb
2 changed files with 40 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
F-Droid/src/org/fdroid/fdroid/RepoUpdater.java
View File

@ -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<Apk> 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);
}
}

14
F-Droid/test/src/org/fdroid/fdroid/RepoUpdaterTest.java
View File

@ -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!