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.
This commit is contained in:
Hans-Christoph Steiner 2015-06-22 12:25:02 -04:00
parent 43cc017c91
commit 2910acc906
2 changed files with 93 additions and 44 deletions

View File

@ -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:
* <li>in the downloaded jar</li>
* <li>in the index XML</li>
* <li>stored in the local database</li>
* 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!");

View File

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