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

update logic to match only parsing signed index files

Browse Source 
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
This commit is contained in:
Hans-Christoph Steiner 2015-06-15 17:40:28 -04:00
parent d7efc99bdb
commit 4f2650cd47
4 changed files with 119 additions and 236 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -18,13 +18,11 @@ import org.xml.sax.InputSource;
import org.xml.sax.SAXException; import org.xml.sax.SAXException;
import org.xml.sax.XMLReader; import org.xml.sax.XMLReader;
import java.io.BufferedReader; import java.io.BufferedInputStream;
import java.io.File; import java.io.File;
import java.io.FileOutputStream;
import java.io.FileReader;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.io.OutputStream; import java.security.CodeSigner;
import java.security.cert.Certificate; import java.security.cert.Certificate;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Date; import java.util.Date;
@ -48,13 +46,18 @@ public class RepoUpdater {
private List<App> apps = new ArrayList<>(); private List<App> apps = new ArrayList<>();
private List<Apk> apks = new ArrayList<>(); private List<Apk> apks = new ArrayList<>();
private RepoUpdateRememberer rememberer = null; private RepoUpdateRememberer rememberer = null;
protected boolean usePubkeyInJar = false;
protected boolean hasChanged = false; protected boolean hasChanged = false;
@Nullable protected ProgressListener progressListener; @Nullable protected ProgressListener progressListener;
public RepoUpdater(@NonNull Context ctx, @NonNull Repo repo) { /**
this.context = ctx; * Updates an app repo as read out of the database into a {@link Repo} instance.
this.repo = repo; *
* @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) { public void setProgressListener(ProgressListener progressListener) {
@ -107,86 +110,74 @@ public class RepoUpdater {
return downloader; return downloader;
} }
private int estimateAppCount(File indexFile) { /**
int count = -1; * All repos are represented by a signed jar file, {@code index.jar}, which contains
try { * a single file, {@code index.xml}. This takes the {@code index.jar}, verifies the
// A bit of a hack, this might return false positives if an apps description * signature, then returns the unzipped {@code index.xml}.
// or some other part of the XML file contains this, but it is a pretty good *
// estimate and makes the progress counter more informative. * @throws UpdateException All error states will come from here.
// 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 = "<application";
count = Utils.countSubstringOccurrence(indexFile, APPLICATION);
} catch (IOException e) {
// Do nothing. Leave count at default -1 value.
}
return count;
}
public void update() throws UpdateException { public void update() throws UpdateException {
File downloadedFile = null; final Downloader downloader = downloadIndex();
File indexFile = null; hasChanged = downloader.hasChanged();
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:
processDownloadedFile(downloader.getFile(), downloader.getCacheTag());
}
}
void processDownloadedFile(File downloadedFile, String cacheTag) throws UpdateException {
InputStream indexInputStream = null;
try { try {
boolean storePubKey = false;
if (repo.pubkey == null) // new repo, no signing certificate stored
storePubKey = true;
final Downloader downloader = downloadIndex(); JarEntry indexEntry = null;
hasChanged = downloader.hasChanged(); if (downloadedFile != null && downloadedFile.exists()) {
JarFile jarFile = new JarFile(downloadedFile, true);
if (hasChanged) { indexEntry = (JarEntry) jarFile.getEntry("index.xml");
indexInputStream = new BufferedInputStream(jarFile.getInputStream(indexEntry));
// 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();
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();
final XMLReader reader = parser.getXMLReader();
final RepoXMLHandler handler = new RepoXMLHandler(repo, progressListener);
if (progressListener != null) {
// Only bother spending the time to count the expected apps
// if we can show that to the user...
handler.setTotalAppCount(estimateAppCount(indexFile));
}
reader.setContentHandler(handler);
InputSource is = new InputSource(
new BufferedReader(new FileReader(indexFile)));
reader.parse(is);
apps = handler.getApps();
apks = handler.getApks();
rememberer = new RepoUpdateRememberer();
rememberer.context = context;
rememberer.repo = repo;
rememberer.values = prepareRepoDetailsForSaving(handler, downloader.getCacheTag());
} }
// Process the index...
final SAXParser parser = SAXParserFactory.newInstance().newSAXParser();
final XMLReader reader = parser.getXMLReader();
final RepoXMLHandler repoXMLHandler = new RepoXMLHandler(repo);
reader.setContentHandler(repoXMLHandler);
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);
apps = repoXMLHandler.getApps();
apks = repoXMLHandler.getApks();
rememberer = new RepoUpdateRememberer();
rememberer.context = context;
rememberer.repo = repo;
rememberer.values = prepareRepoDetailsForSaving(repoXMLHandler, cacheTag, storePubKey);
} catch (SAXException | ParserConfigurationException | IOException e) { } catch (SAXException | ParserConfigurationException | IOException e) {
throw new UpdateException(repo, "Error parsing index for repo " + repo.address, e); throw new UpdateException(repo, "Error parsing index for repo " + repo.address, e);
} finally { } finally {
if (downloadedFile != null && downloadedFile.exists()) { if (indexInputStream != null) {
try {
indexInputStream.close();
} catch (IOException e) {
// ignored
}
}
if (downloadedFile != null) {
downloadedFile.delete(); downloadedFile.delete();
} }
if (indexFile != null && indexFile.exists()) {
indexFile.delete();
}
} }
} }
private ContentValues prepareRepoDetailsForSaving(RepoXMLHandler handler, String etag) { private ContentValues prepareRepoDetailsForSaving(RepoXMLHandler handler, String etag, boolean storePubKey) {
ContentValues values = new ContentValues(); ContentValues values = new ContentValues();
values.put(RepoProvider.DataColumns.LAST_UPDATED, Utils.formatDate(new Date(), "")); values.put(RepoProvider.DataColumns.LAST_UPDATED, Utils.formatDate(new Date(), ""));
@ -199,10 +190,9 @@ public class RepoUpdater {
* We received a repo config that included the fingerprint, so we need to save * We received a repo config that included the fingerprint, so we need to save
* the pubkey now. * the pubkey now.
*/ */
if (handler.getPubKey() != null && (repo.pubkey == null || usePubkeyInJar)) { if (storePubKey && repo.pubkey != null && repo.pubkey.equals(handler.getPubKey())) {
Log.d(TAG, "Public key found - saving in the database."); Log.d(TAG, "Public key found - saving in the database.");
values.put(RepoProvider.DataColumns.PUBLIC_KEY, handler.getPubKey()); values.put(RepoProvider.DataColumns.PUBLIC_KEY, handler.getPubKey());
usePubkeyInJar = false;
} }
if (handler.getVersion() != -1 && handler.getVersion() != repo.version) { if (handler.getVersion() != -1 && handler.getVersion() != repo.version) {
@ -258,100 +248,44 @@ public class RepoUpdater {
} }
} }
private boolean verifyCerts(JarEntry item) throws UpdateException { private void verifyCerts(JarEntry jarEntry, boolean storePubKey) throws UpdateException {
final Certificate[] certs = item.getCertificates(); final CodeSigner[] codeSigners = jarEntry.getCodeSigners();
if (certs == null || certs.length == 0) { if (codeSigners == null || codeSigners.length == 0) {
throw new UpdateException(repo, "No signature found in index"); throw new UpdateException(repo, "No signature found in index");
} }
/* we could in theory support more than 1, but as of now we do not */
if (codeSigners.length > 1) {
throw new UpdateException(repo, "index.jar must be signed by a single code signer!");
}
List<? extends Certificate> 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)"); // though its called repo.pubkey, its actually a X509 certificate
boolean match = false; String pubkey = Hasher.hex(cert);
for (final Certificate cert : certs) {
String certdata = Hasher.hex(cert); /* The first time a repo is added, it can be added with the signing key's
if (repo.pubkey == null && repo.fingerprint != null) { fingerprint. In that case, check that fingerprint against what is
String certFingerprint = Utils.calcFingerprint(cert); actually in the index.jar itself */
Log.d(TAG, "No public key for repo " + repo.address + " yet, but it does have a fingerprint, so comparing them."); if (repo.pubkey == null && repo.fingerprint != null) {
Log.d(TAG, "Repo fingerprint: " + repo.fingerprint); String certFingerprint = Utils.calcFingerprint(cert);
Log.d(TAG, "Cert fingerprint: " + certFingerprint); if (repo.fingerprint.equalsIgnoreCase(certFingerprint)) {
if (repo.fingerprint.equalsIgnoreCase(certFingerprint)) { storePubKey = true;
repo.pubkey = certdata; } else {
usePubkeyInJar = true; throw new UpdateException(repo, "Supplied certificate fingerprint does not match!");
}
}
if (repo.pubkey != null && repo.pubkey.equals(certdata)) {
Log.d(TAG, "Checking repo public key against cert found in jar.");
match = true;
break;
} }
} }
return 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;
* All repos are represented by a signed jar file, {@code index.jar}, which contains return;
* a single file, {@code index.xml}. This takes the {@code index.jar}, verifies the } else if (repo.pubkey != null && repo.pubkey.equals(pubkey)) {
* signature, then returns the unzipped {@code index.xml}. return; // we have a match!
*
* @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
}
}
} }
throw new UpdateException(repo, "Signing certificate does not match!");
return indexFile;
} }
} }

30
F-Droid/src/org/fdroid/fdroid/RepoXMLHandler.java
View File

@ -32,6 +32,9 @@ import org.xml.sax.helpers.DefaultHandler;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
/**
* Parses the index.xml into Java data structures.
*/
public class RepoXMLHandler extends DefaultHandler { public class RepoXMLHandler extends DefaultHandler {
// The repo we're processing. // The repo we're processing.
@ -49,27 +52,18 @@ public class RepoXMLHandler extends DefaultHandler {
private int version = -1; private int version = -1;
private int maxage = -1; private int maxage = -1;
// After processing the XML, this will be null if the index specified a /** the pubkey stored in the header of index.xml */
// 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.
private String pubkey; private String pubkey;
private String name; private String name;
private String description; private String description;
private String hashType; private String hashType;
private int progressCounter = 0; public RepoXMLHandler(Repo repo) {
private final ProgressListener progressListener;
private int totalAppCount;
public RepoXMLHandler(Repo repo, ProgressListener listener) {
this.repo = repo; this.repo = repo;
pubkey = null; pubkey = null;
name = null; name = null;
description = null; description = null;
progressListener = listener;
} }
public List<App> getApps() { return apps; } public List<App> getApps() { return apps; }
@ -265,16 +259,6 @@ public class RepoXMLHandler extends DefaultHandler {
} else if (localName.equals("application") && curapp == null) { } else if (localName.equals("application") && curapp == null) {
curapp = new App(); curapp = new App();
curapp.id = attributes.getValue("", "id"); 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) { } else if (localName.equals("package") && curapp != null && curapk == null) {
curapk = new Apk(); curapk = new Apk();
curapk.id = curapp.id; curapk.id = curapp.id;
@ -287,10 +271,6 @@ public class RepoXMLHandler extends DefaultHandler {
curchars.setLength(0); curchars.setLength(0);
} }
public void setTotalAppCount(int totalAppCount) {
this.totalAppCount = totalAppCount;
}
private String cleanWhiteSpace(String str) { private String cleanWhiteSpace(String str) {
return str.replaceAll("\n", " ").replaceAll(" ", " "); return str.replaceAll("\n", " ").replaceAll(" ", " ");
} }

31
F-Droid/src/org/fdroid/fdroid/Utils.java
View File

@ -45,7 +45,6 @@ import java.io.Closeable;
import java.io.File; import java.io.File;
import java.io.FileInputStream; import java.io.FileInputStream;
import java.io.FileOutputStream; import java.io.FileOutputStream;
import java.io.FileReader;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.io.OutputStream; import java.io.OutputStream;
@ -260,36 +259,6 @@ public final class Utils {
return getMinMaxSdkVersion(context, packageName, "maxSdkVersion"); 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 // return a fingerprint formatted for display
public static String formatFingerprint(Context context, String fingerprint) { public static String formatFingerprint(Context context, String fingerprint) {
if (TextUtils.isEmpty(fingerprint) if (TextUtils.isEmpty(fingerprint)

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

@ -5,12 +5,11 @@ import android.annotation.TargetApi;
import android.content.Context; import android.content.Context;
import android.test.InstrumentationTestCase; import android.test.InstrumentationTestCase;
import org.apache.commons.io.FileUtils;
import org.fdroid.fdroid.RepoUpdater.UpdateException; import org.fdroid.fdroid.RepoUpdater.UpdateException;
import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.Repo;
import java.io.File; import java.io.File;
import java.io.IOException; import java.util.UUID;
@TargetApi(8) @TargetApi(8)
public class RepoUpdaterTest extends InstrumentationTestCase { public class RepoUpdaterTest extends InstrumentationTestCase {
@ -34,17 +33,12 @@ public class RepoUpdaterTest extends InstrumentationTestCase {
public void testExtractIndexFromJar() { public void testExtractIndexFromJar() {
if (!testFilesDir.canWrite()) if (!testFilesDir.canWrite())
return; return;
File simpleIndexXml = TestUtils.copyAssetToDir(context, "simpleIndex.xml", testFilesDir);
File simpleIndexJar = TestUtils.copyAssetToDir(context, "simpleIndex.jar", testFilesDir); File simpleIndexJar = TestUtils.copyAssetToDir(context, "simpleIndex.jar", testFilesDir);
File testFile = null;
// these are supposed to succeed // these are supposed to succeed
try { try {
testFile = repoUpdater.getIndexFromJar(simpleIndexJar); repoUpdater.processDownloadedFile(simpleIndexJar, UUID.randomUUID().toString());
assertTrue(testFile.length() == simpleIndexXml.length()); } catch (UpdateException e) {
assertEquals(FileUtils.readFileToString(testFile),
FileUtils.readFileToString(simpleIndexXml));
} catch (IOException | UpdateException e) {
e.printStackTrace(); e.printStackTrace();
fail(); fail();
} }
@ -55,7 +49,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase {
return; return;
// this is supposed to fail // this is supposed to fail
try { try {
repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithoutSignature.jar", testFilesDir)); File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithoutSignature.jar", testFilesDir);
repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString());
fail(); fail();
} catch (UpdateException e) { } catch (UpdateException e) {
// success! // success!
@ -67,7 +62,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase {
return; return;
// this is supposed to fail // this is supposed to fail
try { try {
repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedManifest.jar", testFilesDir)); File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedManifest.jar", testFilesDir);
repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString());
fail(); fail();
} catch (UpdateException e) { } catch (UpdateException e) {
e.printStackTrace(); e.printStackTrace();
@ -82,7 +78,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase {
return; return;
// this is supposed to fail // this is supposed to fail
try { try {
repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedSignature.jar", testFilesDir)); File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedSignature.jar", testFilesDir);
repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString());
fail(); fail();
} catch (UpdateException e) { } catch (UpdateException e) {
e.printStackTrace(); e.printStackTrace();
@ -97,7 +94,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase {
return; return;
// this is supposed to fail // this is supposed to fail
try { try {
repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedCertificate.jar", testFilesDir)); File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedCertificate.jar", testFilesDir);
repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString());
fail(); fail();
} catch (UpdateException e) { } catch (UpdateException e) {
e.printStackTrace(); e.printStackTrace();
@ -112,7 +110,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase {
return; return;
// this is supposed to fail // this is supposed to fail
try { try {
repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedEverything.jar", testFilesDir)); File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedEverything.jar", testFilesDir);
repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString());
fail(); fail();
} catch (UpdateException e) { } catch (UpdateException e) {
e.printStackTrace(); e.printStackTrace();
@ -127,7 +126,8 @@ public class RepoUpdaterTest extends InstrumentationTestCase {
return; return;
// this is supposed to fail // this is supposed to fail
try { try {
repoUpdater.getIndexFromJar(TestUtils.copyAssetToDir(context, "masterKeyIndex.jar", testFilesDir)); File jarFile = TestUtils.copyAssetToDir(context, "masterKeyIndex.jar", testFilesDir);
repoUpdater.processDownloadedFile(jarFile, UUID.randomUUID().toString());
fail(); fail();
} catch (UpdateException | SecurityException e) { } catch (UpdateException | SecurityException e) {
// success! // success!