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);