From 38d21cd178031d677cb9b636bbf130d370c16c82 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Wed, 22 Mar 2017 14:13:31 +0100
Subject: [PATCH] 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
---
.../org/fdroid/fdroid/IndexV1Updater.java | 15 +++++++-------
.../fdroid/updater/IndexV1UpdaterTest.java | 20 ++++++++++++++++++-
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java
index 97b82c773..5e702d957 100644
--- a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java
+++ b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java
@@ -167,20 +167,19 @@ public class IndexV1Updater extends RepoUpdater {
return;
}
- /* TODO
- if (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
+ long timestamp = (Long) repoMap.get("timestamp") / 1000;
+
+ if (repo.timestamp > timestamp) {
+ throw new RepoUpdater.UpdateException(repo, "index.jar is older that current index! "
+ + timestamp + " < " + repo.timestamp);
+ }
X509Certificate certificate = getSigningCertFromJar(indexEntry);
verifySigningCertificate(certificate);
Utils.debugLog(TAG, "Repo signature verified, saving app metadata to database.");
// timestamp is absolutely required
- repo.timestamp = (Long) repoMap.get("timestamp");
+ repo.timestamp = timestamp;
// below are optional, can be null
repo.name = getStringRepoValue(repoMap, "name");
repo.icon = getStringRepoValue(repoMap, "icon");
diff --git a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java
index 9967a3113..1bc261142 100644
--- a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java
+++ b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java
@@ -27,7 +27,11 @@ import java.util.jar.JarFile;
import static org.hamcrest.CoreMatchers.containsString;
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)
@RunWith(RobolectricTestRunner.class)
@@ -41,6 +45,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
public void testIndexV1Processing() throws IOException, RepoUpdater.UpdateException {
Preferences.setup(context);
Repo repo = MultiRepoUpdaterTest.createRepo("Testy", TESTY_JAR, context, TESTY_CERT);
+ repo.timestamp = 1481222110;
IndexV1Updater updater = new IndexV1Updater(context, repo);
JarFile jarFile = new JarFile(TestUtils.copyResourceToTempFile(TESTY_JAR), true);
Log.i(TAG, "jarFile " + jarFile);
@@ -63,6 +68,19 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
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)
public void testIndexV1WithBadTestyJarNoManifest() throws IOException, RepoUpdater.UpdateException {
testBadTestyJar("testy.at.or.at_no-MANIFEST.MF_index-v1.jar");