remove support for unsigned repos

This has been discussed quite a bit now.  It is very easy to generate a
signed repo on the server, and supporting unsigned repos adds complexity
and security issues, including "BZ-01-002 TOFU Requests too easy to
recognize and intercept" from the audit.

https://gitlab.com/fdroid/fdroidserver/merge_requests/48
closes #12 https://gitlab.com/fdroid/fdroidclient/issues/12
This commit is contained in:
Hans-Christoph Steiner 2015-05-07 16:52:57 -04:00
parent 1c5256a5d7
commit 157b1e242f
5 changed files with 131 additions and 200 deletions

View File

@ -376,7 +376,7 @@ public class UpdateService extends IntentService implements ProgressListener {
}
sendStatus(STATUS_INFO, getString(R.string.status_connecting_to_repo, repo.address));
RepoUpdater updater = RepoUpdater.createUpdaterFor(getBaseContext(), repo);
RepoUpdater updater = new RepoUpdater(getBaseContext(), repo);
updater.setProgressListener(this);
try {
updater.update();

View File

@ -2,9 +2,12 @@ package org.fdroid.fdroid.updater;
import android.content.ContentValues;
import android.content.Context;
import android.content.pm.PackageManager;
import android.os.Bundle;
import android.util.Log;
import org.fdroid.fdroid.FDroidApp;
import org.fdroid.fdroid.Hasher;
import org.fdroid.fdroid.ProgressListener;
import org.fdroid.fdroid.RepoXMLHandler;
import org.fdroid.fdroid.Utils;
@ -20,31 +23,29 @@ import org.xml.sax.XMLReader;
import java.io.BufferedReader;
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.cert.Certificate;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;
abstract public class RepoUpdater {
public class RepoUpdater {
private static final String TAG = "fdroid.RepoUpdater";
public static final String PROGRESS_TYPE_PROCESS_XML = "processingXml";
public static final String PROGRESS_DATA_REPO_ADDRESS = "repoAddress";
public static RepoUpdater createUpdaterFor(Context ctx, Repo repo) {
if (repo.fingerprint == null && repo.pubkey == null) {
return new UnsignedRepoUpdater(ctx, repo);
}
return new SignedRepoUpdater(ctx, repo);
}
protected final Context context;
protected final Repo repo;
private List<App> apps = new ArrayList<>();
@ -70,15 +71,47 @@ abstract public class RepoUpdater {
public List<Apk> getApks() { return apks; }
/**
* For example, you may want to unzip a jar file to get the index inside,
* or if the file is not compressed, you can just return a reference to
* the downloaded file.
* 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 abstract File getIndexFromFile(File downloadedFile) throws UpdateException;
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));
protected abstract String getIndexAddress();
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;
return repo.address + "/index.jar?client_version=" + versionName;
} catch (PackageManager.NameNotFoundException e) {
e.printStackTrace();
}
return repo.address + "/index.jar";
}
protected Downloader downloadIndex() throws UpdateException {
Downloader downloader = null;
@ -128,7 +161,6 @@ abstract public class RepoUpdater {
return count;
}
public void update() throws UpdateException {
File downloadedFile = null;
@ -261,4 +293,84 @@ abstract public class RepoUpdater {
}
}
private boolean verifyCerts(JarEntry item) throws UpdateException {
final Certificate[] certs = item.getCertificates();
if (certs == null || certs.length == 0) {
throw new UpdateException(repo, "No signature found in index");
}
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;
}
}
return match;
}
protected File extractIndexFromJar(File indexJar) throws UpdateException {
File indexFile = null;
JarFile jarFile = null;
try {
jarFile = new JarFile(indexJar, 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 {
if (jarFile != null) {
try {
jarFile.close();
} catch (IOException ioe) {
// ignore
}
}
}
return indexFile;
}
}

View File

@ -1,153 +0,0 @@
package org.fdroid.fdroid.updater;
import android.content.Context;
import android.content.pm.PackageManager;
import android.util.Log;
import org.fdroid.fdroid.FDroidApp;
import org.fdroid.fdroid.Hasher;
import org.fdroid.fdroid.R;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.data.Repo;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.security.cert.Certificate;
import java.util.Date;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
public class SignedRepoUpdater extends RepoUpdater {
private static final String TAG = "fdroid.SignedRepoUpdater";
public SignedRepoUpdater(Context ctx, Repo repo) {
super(ctx, repo);
}
private boolean verifyCerts(JarEntry item) throws UpdateException {
final Certificate[] certs = item.getCertificates();
if (certs == null || certs.length == 0) {
throw new UpdateException(repo, "No signature found in index");
}
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;
}
}
return match;
}
protected File extractIndexFromJar(File indexJar) throws UpdateException {
File indexFile = null;
JarFile jarFile = null;
try {
jarFile = new JarFile(indexJar, 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 {
if (jarFile != null) {
try {
jarFile.close();
} catch (IOException ioe) {
// ignore
}
}
}
return indexFile;
}
@Override
protected String getIndexAddress() {
try {
String versionName = context.getPackageManager().getPackageInfo(context.getPackageName(), 0).versionName;
return repo.address + "/index.jar?client_version=" + versionName;
} catch (PackageManager.NameNotFoundException e) {
e.printStackTrace();
}
return repo.address + "/index.jar";
}
/**
* As this is a signed repo - we download the jar file,
* check the signature, and extract the index file
*/
@Override
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;
}
}

View File

@ -1,28 +0,0 @@
package org.fdroid.fdroid.updater;
import android.content.Context;
import android.util.Log;
import org.fdroid.fdroid.data.Repo;
import java.io.File;
public class UnsignedRepoUpdater extends RepoUpdater {
private static final String TAG = "fdroid.UnsignedRepoUpdater";
public UnsignedRepoUpdater(Context ctx, Repo repo) {
super(ctx, repo);
}
@Override
protected File getIndexFromFile(File file) throws UpdateException {
Log.d(TAG, "Getting unsigned index from " + getIndexAddress());
return file;
}
@Override
protected String getIndexAddress() {
return repo.address + "/index.xml";
}
}

View File

@ -14,8 +14,8 @@ import java.io.File;
import java.io.IOException;
@TargetApi(8)
public class SignedRepoUpdaterTest extends InstrumentationTestCase {
private static final String TAG = "SignedRepoUpdaterTest";
public class RepoUpdaterTest extends InstrumentationTestCase {
private static final String TAG = "RepoUpdaterTest";
private Context context;
private RepoUpdater repoUpdater;
@ -29,7 +29,7 @@ public class SignedRepoUpdaterTest extends InstrumentationTestCase {
testFilesDir = TestUtils.getWriteableDir(getInstrumentation());
Repo repo = new Repo();
repo.pubkey = this.simpleIndexPubkey;
repoUpdater = RepoUpdater.createUpdaterFor(context, repo);
repoUpdater = new RepoUpdater(context, repo);
}
public void testExtractIndexFromJar() {