check timestamp with index-v1

An important security protection is erroring when the index-v1.jar is
older than what is currently in the database.  If the current or older
jar is allowed to be parsed, then a malicious server or
Man-In-The-Middle could replay old version of the index-v1.jar to
prevent the clients from learning about updates that fix security issues
This commit is contained in:
Hans-Christoph Steiner 2017-03-22 14:13:31 +01:00 committed by Peter Serwylo
parent 8cfe1d3584
commit 38d21cd178
2 changed files with 26 additions and 9 deletions

View File

@ -167,20 +167,19 @@ public class IndexV1Updater extends RepoUpdater {
return; return;
} }
/* TODO long timestamp = (Long) repoMap.get("timestamp") / 1000;
if (timestamp < repo.timestamp) {
throw new RepoUpdater.UpdateException(repo, "index.jar is older that current index! " if (repo.timestamp > timestamp) {
+ timestamp + " < " + repo.timestamp); throw new RepoUpdater.UpdateException(repo, "index.jar is older that current index! "
} + timestamp + " < " + repo.timestamp);
*/ }
// TODO handle maxage, convert to "expiration" Date instance
X509Certificate certificate = getSigningCertFromJar(indexEntry); X509Certificate certificate = getSigningCertFromJar(indexEntry);
verifySigningCertificate(certificate); verifySigningCertificate(certificate);
Utils.debugLog(TAG, "Repo signature verified, saving app metadata to database."); Utils.debugLog(TAG, "Repo signature verified, saving app metadata to database.");
// timestamp is absolutely required // timestamp is absolutely required
repo.timestamp = (Long) repoMap.get("timestamp"); repo.timestamp = timestamp;
// below are optional, can be null // below are optional, can be null
repo.name = getStringRepoValue(repoMap, "name"); repo.name = getStringRepoValue(repoMap, "name");
repo.icon = getStringRepoValue(repoMap, "icon"); repo.icon = getStringRepoValue(repoMap, "icon");

View File

@ -27,7 +27,11 @@ import java.util.jar.JarFile;
import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.core.IsNot.not; import static org.hamcrest.core.IsNot.not;
import static org.junit.Assert.*; import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
@Config(constants = BuildConfig.class, sdk = 24) @Config(constants = BuildConfig.class, sdk = 24)
@RunWith(RobolectricTestRunner.class) @RunWith(RobolectricTestRunner.class)
@ -41,6 +45,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
public void testIndexV1Processing() throws IOException, RepoUpdater.UpdateException { public void testIndexV1Processing() throws IOException, RepoUpdater.UpdateException {
Preferences.setup(context); Preferences.setup(context);
Repo repo = MultiRepoUpdaterTest.createRepo("Testy", TESTY_JAR, context, TESTY_CERT); Repo repo = MultiRepoUpdaterTest.createRepo("Testy", TESTY_JAR, context, TESTY_CERT);
repo.timestamp = 1481222110;
IndexV1Updater updater = new IndexV1Updater(context, repo); IndexV1Updater updater = new IndexV1Updater(context, repo);
JarFile jarFile = new JarFile(TestUtils.copyResourceToTempFile(TESTY_JAR), true); JarFile jarFile = new JarFile(TestUtils.copyResourceToTempFile(TESTY_JAR), true);
Log.i(TAG, "jarFile " + jarFile); Log.i(TAG, "jarFile " + jarFile);
@ -63,6 +68,19 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
getClass().getResourceAsStream("foo"); getClass().getResourceAsStream("foo");
} }
@Test(expected = RepoUpdater.UpdateException.class)
public void testIndexV1WithOldTimestamp() throws IOException, RepoUpdater.UpdateException {
Repo repo = MultiRepoUpdaterTest.createRepo("Testy", TESTY_JAR, context, TESTY_CERT);
repo.timestamp = System.currentTimeMillis() / 1000;
IndexV1Updater updater = new IndexV1Updater(context, repo);
JarFile jarFile = new JarFile(TestUtils.copyResourceToTempFile(TESTY_JAR), true);
JarEntry indexEntry = (JarEntry) jarFile.getEntry(IndexV1Updater.DATA_FILE_NAME);
InputStream indexInputStream = jarFile.getInputStream(indexEntry);
updater.processIndexV1(indexInputStream, indexEntry, "fakeEtag");
fail(); // it should never reach here, it should throw a SigningException
getClass().getResourceAsStream("foo");
}
@Test(expected = RepoUpdater.SigningException.class) @Test(expected = RepoUpdater.SigningException.class)
public void testIndexV1WithBadTestyJarNoManifest() throws IOException, RepoUpdater.UpdateException { public void testIndexV1WithBadTestyJarNoManifest() throws IOException, RepoUpdater.UpdateException {
testBadTestyJar("testy.at.or.at_no-MANIFEST.MF_index-v1.jar"); testBadTestyJar("testy.at.or.at_no-MANIFEST.MF_index-v1.jar");