From f7c043b3fc850f3d825d151f42722cdaee105360 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 19 May 2016 08:27:50 +0200 Subject: [PATCH 1/5] set versionName based on git release tag This makes it easy to tell which debug build a device is running, since the versionName now automatically describes the exact commit that was built, based on `git describe`, e.g.: 0.100-alpha7-33-gc2e8e8a For release builds, i.e. builds from commits that are tagged as releases, the versionName will be just the tag name: 0.100-alpha8 closes #664 https://gitlab.com/fdroid/fdroidclient/issues/664 --- app/build.gradle | 11 +++++++++++ app/src/main/AndroidManifest.xml | 4 +--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 303045455..d668052a5 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -3,6 +3,16 @@ apply plugin: 'witness' apply plugin: 'checkstyle' apply plugin: 'pmd' +/* gets the version name from the latest Git tag, stripping the leading v off */ +def getVersionName = { -> + def stdout = new ByteArrayOutputStream() + exec { + commandLine 'git', 'describe', '--tags', '--always' + standardOutput = stdout + } + return stdout.toString().trim().substring(1) +} + repositories { jcenter() } @@ -150,6 +160,7 @@ android { } defaultConfig { + versionName getVersionName() testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner" } diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 4551f1095..7978e0807 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -2,9 +2,7 @@ + android:versionCode="100007"> Date: Thu, 19 May 2016 09:08:51 +0200 Subject: [PATCH 2/5] use Environment.getRootDirectory() instead of /system Its officially possible to have the ROM's filesystem with any name. While I have never seen that in practice, Android does provide an easy method to get the real name. Plus this should help avoid typos and the like, and make it easy to track things that rely on that filesystem path. --- .../main/java/org/fdroid/fdroid/FDroidApp.java | 3 +++ .../java/org/fdroid/fdroid/compat/FileCompat.java | 5 +++-- .../privileged/install/InstallExtension.java | 15 ++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index 9d9c19eed..dd7aa9a15 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -34,6 +34,7 @@ import android.content.pm.ResolveInfo; import android.content.res.Configuration; import android.net.Uri; import android.os.Build; +import android.os.Environment; import android.os.StrictMode; import android.preference.PreferenceManager; import android.text.TextUtils; @@ -78,6 +79,8 @@ public class FDroidApp extends Application { private static final String TAG = "FDroidApp"; + public static final String SYSTEM_DIR_NAME = Environment.getRootDirectory().getAbsolutePath(); + private static Locale locale; // for the local repo on this device, all static since there is only one diff --git a/app/src/main/java/org/fdroid/fdroid/compat/FileCompat.java b/app/src/main/java/org/fdroid/fdroid/compat/FileCompat.java index cb8726a53..663d7ba0a 100644 --- a/app/src/main/java/org/fdroid/fdroid/compat/FileCompat.java +++ b/app/src/main/java/org/fdroid/fdroid/compat/FileCompat.java @@ -5,6 +5,7 @@ import android.os.Build; import android.system.ErrnoException; import android.util.Log; +import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.SanitizedFile; @@ -59,7 +60,7 @@ public class FileCompat { protected static void symlinkRuntime(SanitizedFile source, SanitizedFile dest) { String[] commands = { - "/system/bin/ln", + FDroidApp.SYSTEM_DIR_NAME + "/bin/ln", source.getAbsolutePath(), dest.getAbsolutePath(), }; @@ -107,7 +108,7 @@ public class FileCompat { // The "file" must be a sanitized file, and hence only contain A-Za-z0-9.-_ already, // but it makes no assurances about the parent directory. final String[] args = { - "/system/bin/chmod", + FDroidApp.SYSTEM_DIR_NAME + "/bin/chmod", mode, file.getAbsolutePath(), }; diff --git a/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtension.java b/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtension.java index bfc3a99ec..3ea179fec 100644 --- a/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtension.java +++ b/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtension.java @@ -22,6 +22,7 @@ package org.fdroid.fdroid.privileged.install; import android.content.Context; import android.os.Build; +import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.R; import org.fdroid.fdroid.installer.PrivilegedInstaller; @@ -85,11 +86,11 @@ abstract class InstallExtension { private List getInstallCommands(String apkPath) { final List commands = new ArrayList<>(); - commands.add("mount -o rw,remount /system"); // remount as read-write + commands.add("mount -o rw,remount " + FDroidApp.SYSTEM_DIR_NAME); // remount as read-write commands.addAll(getCopyToSystemCommands(apkPath)); commands.add("mv " + getInstallPath() + ".tmp " + getInstallPath()); commands.add("sleep 5"); // wait until the app is really installed - commands.add("mount -o ro,remount /system"); // remount as read-only + commands.add("mount -o ro,remount " + FDroidApp.SYSTEM_DIR_NAME); // remount as read-only commands.add("am force-stop " + PrivilegedInstaller.PRIVILEGED_EXTENSION_PACKAGE_NAME); commands.addAll(getPostInstallCommands()); return commands; @@ -113,10 +114,10 @@ abstract class InstallExtension { final List commands = new ArrayList<>(); commands.add("am force-stop " + PrivilegedInstaller.PRIVILEGED_EXTENSION_PACKAGE_NAME); commands.add("pm clear " + PrivilegedInstaller.PRIVILEGED_EXTENSION_PACKAGE_NAME); - commands.add("mount -o rw,remount /system"); + commands.add("mount -o rw,remount " + FDroidApp.SYSTEM_DIR_NAME); commands.addAll(getCleanUninstallCommands()); commands.add("sleep 5"); - commands.add("mount -o ro,remount /system"); + commands.add("mount -o ro,remount " + FDroidApp.SYSTEM_DIR_NAME); commands.addAll(getPostUninstallCommands()); return commands; } @@ -139,7 +140,7 @@ abstract class InstallExtension { @Override protected String getSystemFolder() { - return "/system/app/"; + return FDroidApp.SYSTEM_DIR_NAME + "/app/"; } } @@ -156,7 +157,7 @@ abstract class InstallExtension { */ @Override protected String getSystemFolder() { - return "/system/priv-app/"; + return FDroidApp.SYSTEM_DIR_NAME + "/priv-app/"; } } @@ -190,7 +191,7 @@ abstract class InstallExtension { */ @Override protected String getSystemFolder() { - return "/system/priv-app/FDroidPrivileged/"; + return FDroidApp.SYSTEM_DIR_NAME + "/priv-app/FDroidPrivileged/"; } /** From 02b2090e530ab5d22b522d978728f34bb332b390 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 19 May 2016 12:30:44 +0200 Subject: [PATCH 3/5] check repo index timestamps to prevent rollback attacks A hacked fdroid server could "replay" old index.jar files known to have apps with vulnerabilities in it. That provides a long window of time for exploiting that vulnerability. By checking that the timestamp of an update is never older than the current index, this attack is prevented. --- .../org/fdroid/fdroid/RepoUpdaterTest.java | 24 +++++++++++++++++-- .../org/fdroid/fdroid/RepoXMLHandlerTest.java | 9 ++++++- .../java/org/fdroid/fdroid/RepoUpdater.java | 16 ++++++++++--- .../org/fdroid/fdroid/RepoXMLHandler.java | 21 +++++++++++++--- .../java/org/fdroid/fdroid/data/DBHelper.java | 22 +++++++++++++++-- .../java/org/fdroid/fdroid/data/Repo.java | 12 +++++++++- .../org/fdroid/fdroid/data/RepoProvider.java | 3 ++- .../fdroid/localrepo/LocalRepoManager.java | 1 + 8 files changed, 95 insertions(+), 13 deletions(-) diff --git a/app/src/androidTest/java/org/fdroid/fdroid/RepoUpdaterTest.java b/app/src/androidTest/java/org/fdroid/fdroid/RepoUpdaterTest.java index 9566bd54d..7b9012800 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/RepoUpdaterTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/RepoUpdaterTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.fail; public class RepoUpdaterTest { private Context context; + private Repo repo; private RepoUpdater repoUpdater; private File testFilesDir; @@ -30,10 +31,9 @@ public class RepoUpdaterTest { Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); context = instrumentation.getContext(); testFilesDir = TestUtils.getWriteableDir(instrumentation); - Repo repo = new Repo(); + repo = new Repo(); repo.address = "https://fake.url/fdroid/repo"; repo.signingCertificate = this.simpleIndexSigningCert; - repoUpdater = new RepoUpdater(context, repo); } @Test @@ -42,6 +42,7 @@ public class RepoUpdaterTest { return; } File simpleIndexJar = TestUtils.copyAssetToDir(context, "simpleIndex.jar", testFilesDir); + repoUpdater = new RepoUpdater(context, repo); // these are supposed to succeed try { @@ -52,6 +53,18 @@ public class RepoUpdaterTest { } } + @Test(expected = UpdateException.class) + public void testExtractIndexFromOutdatedJar() throws UpdateException { + File simpleIndexJar = TestUtils.copyAssetToDir(context, "simpleIndex.jar", testFilesDir); + repo.version = 10; + repo.timestamp = System.currentTimeMillis() / 1000L; + repoUpdater = new RepoUpdater(context, repo); + + // these are supposed to fail + repoUpdater.processDownloadedFile(simpleIndexJar); + fail(); + } + @Test(expected = UpdateException.class) public void testExtractIndexFromJarWithoutSignatureJar() throws UpdateException { if (!testFilesDir.canWrite()) { @@ -59,7 +72,9 @@ public class RepoUpdaterTest { } // this is supposed to fail File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithoutSignature.jar", testFilesDir); + repoUpdater = new RepoUpdater(context, repo); repoUpdater.processDownloadedFile(jarFile); + fail(); } @Test @@ -70,6 +85,7 @@ public class RepoUpdaterTest { // this is supposed to fail try { File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedManifest.jar", testFilesDir); + repoUpdater = new RepoUpdater(context, repo); repoUpdater.processDownloadedFile(jarFile); fail(); } catch (UpdateException e) { @@ -88,6 +104,7 @@ public class RepoUpdaterTest { // this is supposed to fail try { File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedSignature.jar", testFilesDir); + repoUpdater = new RepoUpdater(context, repo); repoUpdater.processDownloadedFile(jarFile); fail(); } catch (UpdateException e) { @@ -106,6 +123,7 @@ public class RepoUpdaterTest { // this is supposed to fail try { File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedCertificate.jar", testFilesDir); + repoUpdater = new RepoUpdater(context, repo); repoUpdater.processDownloadedFile(jarFile); fail(); } catch (UpdateException e) { @@ -124,6 +142,7 @@ public class RepoUpdaterTest { // this is supposed to fail try { File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedEverything.jar", testFilesDir); + repoUpdater = new RepoUpdater(context, repo); repoUpdater.processDownloadedFile(jarFile); fail(); } catch (UpdateException e) { @@ -142,6 +161,7 @@ public class RepoUpdaterTest { // this is supposed to fail try { File jarFile = TestUtils.copyAssetToDir(context, "masterKeyIndex.jar", testFilesDir); + repoUpdater = new RepoUpdater(context, repo); repoUpdater.processDownloadedFile(jarFile); fail(); //NOPMD } catch (UpdateException e) { diff --git a/app/src/androidTest/java/org/fdroid/fdroid/RepoXMLHandlerTest.java b/app/src/androidTest/java/org/fdroid/fdroid/RepoXMLHandlerTest.java index e4838d289..8b2b7f6f1 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/RepoXMLHandlerTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/RepoXMLHandlerTest.java @@ -43,6 +43,7 @@ public class RepoXMLHandlerTest { expectedRepo.name = "F-Droid"; expectedRepo.signingCertificate = "308201ee30820157a0030201020204300d845b300d06092a864886f70d01010b0500302a3110300e060355040b1307462d44726f6964311630140603550403130d70616c6174736368696e6b656e301e170d3134303432373030303633315a170d3431303931323030303633315a302a3110300e060355040b1307462d44726f6964311630140603550403130d70616c6174736368696e6b656e30819f300d06092a864886f70d010101050003818d0030818902818100a439472e4b6d01141bfc94ecfe131c7c728fdda670bb14c57ca60bd1c38a8b8bc0879d22a0a2d0bc0d6fdd4cb98d1d607c2caefbe250a0bd0322aedeb365caf9b236992fac13e6675d3184a6c7c6f07f73410209e399a9da8d5d7512bbd870508eebacff8b57c3852457419434d34701ccbf692267cbc3f42f1c5d1e23762d790203010001a321301f301d0603551d0e041604140b1840691dab909746fde4bfe28207d1cae15786300d06092a864886f70d01010b05000381810062424c928ffd1b6fd419b44daafef01ca982e09341f7077fb865905087aeac882534b3bd679b51fdfb98892cef38b63131c567ed26c9d5d9163afc775ac98ad88c405d211d6187bde0b0d236381cc574ba06ef9080721a92ae5a103a7301b2c397eecc141cc850dd3e123813ebc41c59d31ddbcb6e984168280c53272f6a442b"; expectedRepo.description = "The official repository of the F-Droid client. Applications in this repository are either official binaries built by the original application developers, or are binaries built from source by the admin of f-droid.org using the tools on https://gitorious.org/f-droid."; + expectedRepo.timestamp = 1398733213; RepoDetails actualDetails = getFromFile("simpleIndex.xml"); handlerTestSuite(expectedRepo, actualDetails, 0, 0, -1, 12); } @@ -53,6 +54,7 @@ public class RepoXMLHandlerTest { expectedRepo.name = "Android-Nexus-7-20139453 on UNSET"; expectedRepo.signingCertificate = "308202da308201c2a00302010202080eb08c796fec91aa300d06092a864886f70d0101050500302d3111300f060355040a0c084b6572706c61707031183016060355040b0c0f477561726469616e50726f6a656374301e170d3134313030333135303631325a170d3135313030333135303631325a302d3111300f060355040a0c084b6572706c61707031183016060355040b0c0f477561726469616e50726f6a65637430820122300d06092a864886f70d01010105000382010f003082010a0282010100c7ab44b130be5c00eedcc3625462f6f6ac26e502641cd641f3e30cbb0ff1ba325158611e7fc2448a35b6a6df30dc6e23602cf6909448befcf11e2fe486b580f1e76fe5887d159050d00afd2c4079f6538896bb200627f4b3e874f011ce5df0fef5d150fcb0b377b531254e436eaf4083ea72fe3b8c3ef450789fa858f2be8f6c5335bb326aff3dda689fbc7b5ba98dea53651dbea7452c38d294985ac5dd8a9e491a695de92c706d682d6911411fcaef3b0a08a030fe8a84e47acaab0b7edcda9d190ce39e810b79b1d8732eca22b15f0d048c8d6f00503a7ee81ab6e08919ff465883432304d95238b95e95c5f74e0a421809e2a6a85825aed680e0d6939e8f0203010001300d06092a864886f70d010105050003820101006d17aad3271b8b2c299dbdb7b1182849b0d5ddb9f1016dcb3487ae0db02b6be503344c7d066e2050bcd01d411b5ee78c7ed450f0ff9da5ce228f774cbf41240361df53d9c6078159d16f4d34379ab7dedf6186489397c83b44b964251a2ebb42b7c4689a521271b1056d3b5a5fa8f28ba64fb8ce5e2226c33c45d27ba3f632dc266c12abf582b8438c2abcf3eae9de9f31152b4158ace0ef33435c20eb809f1b3988131db6e5a1442f2617c3491d9565fedb3e320e8df4236200d3bd265e47934aa578f84d0d1a5efeb49b39907e876452c46996d0feff9404b41aa5631b4482175d843d5512ded45e12a514690646492191e7add434afce63dbff8f0b03ec0c"; expectedRepo.description = "A local FDroid repo generated from apps installed on Android-Nexus-7-20139453"; + expectedRepo.timestamp = 1412696461; RepoDetails actualDetails = getFromFile("smallRepo.xml"); handlerTestSuite(expectedRepo, actualDetails, 12, 12, 14, -1); checkIncludedApps(actualDetails.apps, new String[]{ @@ -77,6 +79,7 @@ public class RepoXMLHandlerTest { expectedRepo.name = "Guardian Project Official Releases"; expectedRepo.signingCertificate = "308205d8308203c0020900a397b4da7ecda034300d06092a864886f70d01010505003081ad310b30090603550406130255533111300f06035504080c084e657720596f726b3111300f06035504070c084e657720596f726b31143012060355040b0c0b4644726f6964205265706f31193017060355040a0c10477561726469616e2050726f6a656374311d301b06035504030c14677561726469616e70726f6a6563742e696e666f3128302606092a864886f70d0109011619726f6f7440677561726469616e70726f6a6563742e696e666f301e170d3134303632363139333931385a170d3431313131303139333931385a3081ad310b30090603550406130255533111300f06035504080c084e657720596f726b3111300f06035504070c084e657720596f726b31143012060355040b0c0b4644726f6964205265706f31193017060355040a0c10477561726469616e2050726f6a656374311d301b06035504030c14677561726469616e70726f6a6563742e696e666f3128302606092a864886f70d0109011619726f6f7440677561726469616e70726f6a6563742e696e666f30820222300d06092a864886f70d01010105000382020f003082020a0282020100b3cd79121b9b883843be3c4482e320809106b0a23755f1dd3c7f46f7d315d7bb2e943486d61fc7c811b9294dcc6b5baac4340f8db2b0d5e14749e7f35e1fc211fdbc1071b38b4753db201c314811bef885bd8921ad86facd6cc3b8f74d30a0b6e2e6e576f906e9581ef23d9c03e926e06d1f033f28bd1e21cfa6a0e3ff5c9d8246cf108d82b488b9fdd55d7de7ebb6a7f64b19e0d6b2ab1380a6f9d42361770d1956701a7f80e2de568acd0bb4527324b1e0973e89595d91c8cc102d9248525ae092e2c9b69f7414f724195b81427f28b1d3d09a51acfe354387915fd9521e8c890c125fc41a12bf34d2a1b304067ab7251e0e9ef41833ce109e76963b0b256395b16b886bca21b831f1408f836146019e7908829e716e72b81006610a2af08301de5d067c9e114a1e5759db8a6be6a3cc2806bcfe6fafd41b5bc9ddddb3dc33d6f605b1ca7d8a9e0ecdd6390d38906649e68a90a717bea80fa220170eea0c86fc78a7e10dac7b74b8e62045a3ecca54e035281fdc9fe5920a855fde3c0be522e3aef0c087524f13d973dff3768158b01a5800a060c06b451ec98d627dd052eda804d0556f60dbc490d94e6e9dea62ffcafb5beffbd9fc38fb2f0d7050004fe56b4dda0a27bc47554e1e0a7d764e17622e71f83a475db286bc7862deee1327e2028955d978272ea76bf0b88e70a18621aba59ff0c5993ef5f0e5d6b6b98e68b70203010001300d06092a864886f70d0101050500038202010079c79c8ef408a20d243d8bd8249fb9a48350dc19663b5e0fce67a8dbcb7de296c5ae7bbf72e98a2020fb78f2db29b54b0e24b181aa1c1d333cc0303685d6120b03216a913f96b96eb838f9bff125306ae3120af838c9fc07ebb5100125436bd24ec6d994d0bff5d065221871f8410daf536766757239bf594e61c5432c9817281b985263bada8381292e543a49814061ae11c92a316e7dc100327b59e3da90302c5ada68c6a50201bda1fcce800b53f381059665dbabeeb0b50eb22b2d7d2d9b0aa7488ca70e67ac6c518adb8e78454a466501e89d81a45bf1ebc350896f2c3ae4b6679ecfbf9d32960d4f5b493125c7876ef36158562371193f600bc511000a67bdb7c664d018f99d9e589868d103d7e0994f166b2ba18ff7e67d8c4da749e44dfae1d930ae5397083a51675c409049dfb626a96246c0015ca696e94ebb767a20147834bf78b07fece3f0872b057c1c519ff882501995237d8206b0b3832f78753ebd8dcbd1d3d9f5ba733538113af6b407d960ec4353c50eb38ab29888238da843cd404ed8f4952f59e4bbc0035fc77a54846a9d419179c46af1b4a3b7fc98e4d312aaa29b9b7d79e739703dc0fa41c7280d5587709277ffa11c3620f5fba985b82c238ba19b17ebd027af9424be0941719919f620dd3bb3c3f11638363708aa11f858e153cf3a69bce69978b90e4a273836100aa1e617ba455cd00426847f"; expectedRepo.description = "The official app repository of The Guardian Project. Applications in this repository are official binaries build by the original application developers and signed by the same key as the APKs that are released in the Google Play store."; + expectedRepo.timestamp = 1411427879; RepoDetails actualDetails = getFromFile("mediumRepo.xml"); handlerTestSuite(expectedRepo, actualDetails, 15, 36, 60, 12); checkIncludedApps(actualDetails.apps, new String[]{ @@ -104,6 +107,7 @@ public class RepoXMLHandlerTest { expectedRepo.name = "F-Droid"; expectedRepo.signingCertificate = "3082035e30820246a00302010202044c49cd00300d06092a864886f70d01010505003071310b300906035504061302554b3110300e06035504081307556e6b6e6f776e3111300f0603550407130857657468657262793110300e060355040a1307556e6b6e6f776e3110300e060355040b1307556e6b6e6f776e311930170603550403131043696172616e2047756c746e69656b73301e170d3130303732333137313032345a170d3337313230383137313032345a3071310b300906035504061302554b3110300e06035504081307556e6b6e6f776e3111300f0603550407130857657468657262793110300e060355040a1307556e6b6e6f776e3110300e060355040b1307556e6b6e6f776e311930170603550403131043696172616e2047756c746e69656b7330820122300d06092a864886f70d01010105000382010f003082010a028201010096d075e47c014e7822c89fd67f795d23203e2a8843f53ba4e6b1bf5f2fd0e225938267cfcae7fbf4fe596346afbaf4070fdb91f66fbcdf2348a3d92430502824f80517b156fab00809bdc8e631bfa9afd42d9045ab5fd6d28d9e140afc1300917b19b7c6c4df4a494cf1f7cb4a63c80d734265d735af9e4f09455f427aa65a53563f87b336ca2c19d244fcbba617ba0b19e56ed34afe0b253ab91e2fdb1271f1b9e3c3232027ed8862a112f0706e234cf236914b939bcf959821ecb2a6c18057e070de3428046d94b175e1d89bd795e535499a091f5bc65a79d539a8d43891ec504058acb28c08393b5718b57600a211e803f4a634e5c57f25b9b8c4422c6fd90203010001300d06092a864886f70d0101050500038201010008e4ef699e9807677ff56753da73efb2390d5ae2c17e4db691d5df7a7b60fc071ae509c5414be7d5da74df2811e83d3668c4a0b1abc84b9fa7d96b4cdf30bba68517ad2a93e233b042972ac0553a4801c9ebe07bf57ebe9a3b3d6d663965260e50f3b8f46db0531761e60340a2bddc3426098397fda54044a17e5244549f9869b460ca5e6e216b6f6a2db0580b480ca2afe6ec6b46eedacfa4aa45038809ece0c5978653d6c85f678e7f5a2156d1bedd8117751e64a4b0dcd140f3040b021821a8d93aed8d01ba36db6c82372211fed714d9a32607038cdfd565bd529ffc637212aaa2c224ef22b603eccefb5bf1e085c191d4b24fe742b17ab3f55d4e6f05ef"; expectedRepo.description = "The official FDroid repository. Applications in this repository are mostly built directory from the source code. Some are official binaries built by the original application developers - these will be replaced by source-built versions over time."; + expectedRepo.timestamp = 1412746769; RepoDetails actualDetails = getFromFile("largeRepo.xml"); handlerTestSuite(expectedRepo, actualDetails, 1211, 2381, 14, 12); /* @@ -626,6 +630,7 @@ public class RepoXMLHandlerTest { assertEquals(actualDetails.maxAge, maxAge); assertEquals(actualDetails.version, version); + assertEquals(expectedRepo.timestamp, actualDetails.timestamp); List apps = actualDetails.apps; assertNotNull(apps); @@ -643,17 +648,19 @@ public class RepoXMLHandlerTest { public String signingCert; public int maxAge; public int version; + public long timestamp; public List apks = new ArrayList<>(); public List apps = new ArrayList<>(); @Override - public void receiveRepo(String name, String description, String signingCert, int maxage, int version) { + public void receiveRepo(String name, String description, String signingCert, int maxage, int version, long timestamp) { this.name = name; this.description = description; this.signingCert = signingCert; this.maxAge = maxage; this.version = version; + this.timestamp = timestamp; } @Override diff --git a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java index d626e2983..941419b2a 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java @@ -156,9 +156,9 @@ public class RepoUpdater { private RepoXMLHandler.IndexReceiver createIndexReceiver() { return new RepoXMLHandler.IndexReceiver() { @Override - public void receiveRepo(String name, String description, String signingCert, int maxAge, int version) { + public void receiveRepo(String name, String description, String signingCert, int maxAge, int version, long timestamp) { signingCertFromIndexXml = signingCert; - repoDetailsToSave = prepareRepoDetailsForSaving(name, description, maxAge, version); + repoDetailsToSave = prepareRepoDetailsForSaving(name, description, maxAge, version, timestamp); } @Override @@ -196,6 +196,12 @@ public class RepoUpdater { reader.setContentHandler(repoXMLHandler); reader.parse(new InputSource(indexInputStream)); + long timestamp = repoDetailsToSave.getAsLong("timestamp"); + if (timestamp < repo.timestamp) { + throw new UpdateException(repo, "index.jar is older that current index! " + + timestamp + " < " + repo.timestamp); + } + signingCertFromJar = getSigningCertFromJar(indexEntry); // JarEntry can only read certificates after the file represented by that JarEntry @@ -242,7 +248,7 @@ public class RepoUpdater { * Update tracking data for the repo represented by this instance (index version, etag, * description, human-readable name, etc. */ - private ContentValues prepareRepoDetailsForSaving(String name, String description, int maxAge, int version) { + private ContentValues prepareRepoDetailsForSaving(String name, String description, int maxAge, int version, long timestamp) { ContentValues values = new ContentValues(); values.put(RepoProvider.DataColumns.LAST_UPDATED, Utils.formatTime(new Date(), "")); @@ -269,6 +275,10 @@ public class RepoUpdater { values.put(RepoProvider.DataColumns.NAME, name); } + if (timestamp != repo.timestamp) { + values.put(RepoProvider.DataColumns.TIMESTAMP, timestamp); + } + return values; } diff --git a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java index e5469633d..ca2d5ed9c 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java @@ -51,6 +51,7 @@ public class RepoXMLHandler extends DefaultHandler { // them - otherwise it will be the value specified. private int repoMaxAge = -1; private int repoVersion; + private long repoTimestamp; private String repoDescription; private String repoName; @@ -60,7 +61,7 @@ public class RepoXMLHandler extends DefaultHandler { private final StringBuilder curchars = new StringBuilder(); interface IndexReceiver { - void receiveRepo(String name, String description, String signingCert, int maxage, int version); + void receiveRepo(String name, String description, String signingCert, int maxage, int version, long timestamp); void receiveApp(App app, List packages); } @@ -79,7 +80,7 @@ public class RepoXMLHandler extends DefaultHandler { @Override public void endElement(String uri, String localName, String qName) - throws SAXException { + throws SAXException { if ("application".equals(localName) && curapp != null) { onApplicationParsed(); @@ -239,7 +240,7 @@ public class RepoXMLHandler extends DefaultHandler { } private void onRepoParsed() { - receiver.receiveRepo(repoName, repoDescription, repoSigningCert, repoMaxAge, repoVersion); + receiver.receiveRepo(repoName, repoDescription, repoSigningCert, repoMaxAge, repoVersion, repoTimestamp); } @Override @@ -253,6 +254,7 @@ public class RepoXMLHandler extends DefaultHandler { repoVersion = Utils.parseInt(attributes.getValue("", "version"), -1); repoName = cleanWhiteSpace(attributes.getValue("", "name")); repoDescription = cleanWhiteSpace(attributes.getValue("", "description")); + repoTimestamp = parseLong(attributes.getValue("", "timestamp"), 0); } else if ("application".equals(localName) && curapp == null) { curapp = new App(); curapp.packageName = attributes.getValue("", "id"); @@ -271,4 +273,17 @@ public class RepoXMLHandler extends DefaultHandler { private static String cleanWhiteSpace(@Nullable String str) { return str == null ? null : str.replaceAll("\\s", " "); } + + private static long parseLong(String str, long fallback) { + if (str == null || str.length() == 0) { + return fallback; + } + long result; + try { + result = Long.parseLong(str); + } catch (NumberFormatException e) { + result = fallback; + } + return result; + } } diff --git a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java index 073e585b7..72fc8e42a 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -35,7 +35,8 @@ class DBHelper extends SQLiteOpenHelper { + "version integer not null default 0, " + "lastetag text, lastUpdated string," + "isSwap integer boolean default 0," - + "username string, password string" + + "username string, password string," + + "timestamp integer not null default 0" + ");"; private static final String CREATE_TABLE_APK = @@ -106,7 +107,7 @@ class DBHelper extends SQLiteOpenHelper { + " );"; private static final String DROP_TABLE_INSTALLED_APP = "DROP TABLE " + TABLE_INSTALLED_APP + ";"; - private static final int DB_VERSION = 54; + private static final int DB_VERSION = 55; private final Context context; @@ -261,6 +262,7 @@ class DBHelper extends SQLiteOpenHelper { values.put(RepoProvider.DataColumns.IN_USE, inUse); values.put(RepoProvider.DataColumns.PRIORITY, priority); values.put(RepoProvider.DataColumns.LAST_ETAG, (String) null); + values.put(RepoProvider.DataColumns.TIMESTAMP, 0); Utils.debugLog(TAG, "Adding repository " + name); db.insert(TABLE_REPO, null, values); @@ -294,6 +296,7 @@ class DBHelper extends SQLiteOpenHelper { addCredentialsToRepo(db, oldVersion); addAuthorToApp(db, oldVersion); useMaxValueInMaxSdkVersion(db, oldVersion); + requireTimestampInRepos(db, oldVersion); } /** @@ -502,6 +505,21 @@ class DBHelper extends SQLiteOpenHelper { db.update(TABLE_APK, values, ApkProvider.DataColumns.MAX_SDK_VERSION + " < 1", null); } + /** + * The {@code } value was in the metadata for a long time, + * but it was not being used in the client until now. + */ + private void requireTimestampInRepos(SQLiteDatabase db, int oldVersion) { + if (oldVersion >= 55) { + return; + } + if (!columnExists(db, TABLE_REPO, RepoProvider.DataColumns.TIMESTAMP)) { + Utils.debugLog(TAG, "Adding " + RepoProvider.DataColumns.TIMESTAMP + " column to " + TABLE_REPO); + db.execSQL("alter table " + TABLE_REPO + " add column " + + RepoProvider.DataColumns.TIMESTAMP + " integer not null default 0"); + } + } + /** * By clearing the etags stored in the repo table, it means that next time the user updates * their repos (either manually or on a scheduled task), they will update regardless of whether diff --git a/app/src/main/java/org/fdroid/fdroid/data/Repo.java b/app/src/main/java/org/fdroid/fdroid/data/Repo.java index 32f2bb483..481ce9045 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Repo.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Repo.java @@ -26,7 +26,7 @@ public class Repo extends ValueObject { /** The signing certificate, {@code null} for a newly added repo */ public String signingCertificate; /** - * The SHA1 fingerprint of {@link #pubkey}, set to {@code null} when a + * The SHA1 fingerprint of {@link #signingCertificate}, set to {@code null} when a * newly added repo did not include fingerprint. It should never be an * empty {@link String}, i.e. {@code ""} */ public String fingerprint; @@ -40,6 +40,9 @@ public class Repo extends ValueObject { public String username; public String password; + /** When the signed repo index was generated, used to protect against replay attacks */ + public long timestamp; + public Repo() { } @@ -94,6 +97,9 @@ public class Repo extends ValueObject { case RepoProvider.DataColumns.PASSWORD: password = cursor.getString(i); break; + case RepoProvider.DataColumns.TIMESTAMP: + timestamp = cursor.getLong(i); + break; } } } @@ -210,5 +216,9 @@ public class Repo extends ValueObject { if (values.containsKey(RepoProvider.DataColumns.PASSWORD)) { password = values.getAsString(RepoProvider.DataColumns.PASSWORD); } + + if (values.containsKey(RepoProvider.DataColumns.TIMESTAMP)) { + timestamp = toInt(values.getAsInteger(RepoProvider.DataColumns.TIMESTAMP)); + } } } diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java b/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java index 5fe4e1e02..3d1e76ac1 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java @@ -225,11 +225,12 @@ public class RepoProvider extends FDroidProvider { String IS_SWAP = "isSwap"; String USERNAME = "username"; String PASSWORD = "password"; + String TIMESTAMP = "timestamp"; String[] ALL = { _ID, ADDRESS, NAME, DESCRIPTION, IN_USE, PRIORITY, SIGNING_CERT, FINGERPRINT, MAX_AGE, LAST_UPDATED, LAST_ETAG, VERSION, IS_SWAP, - USERNAME, PASSWORD, + USERNAME, PASSWORD, TIMESTAMP, }; } diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java b/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java index f45a92656..5826ba062 100644 --- a/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java +++ b/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java @@ -384,6 +384,7 @@ public final class LocalRepoManager { serializer.attribute("", "pubkey", Hasher.hex(LocalRepoKeyStore.get(context).getCertificate())); long timestamp = System.currentTimeMillis() / 1000L; serializer.attribute("", "timestamp", String.valueOf(timestamp)); + serializer.attribute("", "version", "10"); tag("description", "A local FDroid repo generated from apps installed on " + Preferences.get().getLocalRepoName()); From f1a31a7fe393301441203e5f78a52c42937b33f7 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 20 May 2016 11:13:48 +0200 Subject: [PATCH 4/5] RepoUpdaterTest: convert writable test to JUnit assumption This will mark the test as ignored rather then succeeded if it cannot find a writable dir. --- .../org/fdroid/fdroid/RepoUpdaterTest.java | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/app/src/androidTest/java/org/fdroid/fdroid/RepoUpdaterTest.java b/app/src/androidTest/java/org/fdroid/fdroid/RepoUpdaterTest.java index 7b9012800..ac77cbd60 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/RepoUpdaterTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/RepoUpdaterTest.java @@ -5,6 +5,7 @@ import android.app.Instrumentation; import android.content.Context; import android.support.test.InstrumentationRegistry; import android.support.test.runner.AndroidJUnit4; +import android.util.Log; import org.fdroid.fdroid.RepoUpdater.UpdateException; import org.fdroid.fdroid.data.Repo; @@ -15,9 +16,11 @@ import org.junit.runner.RunWith; import java.io.File; import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; @RunWith(AndroidJUnit4.class) public class RepoUpdaterTest { + public static final String TAG = "RepoUpdaterTest"; private Context context; private Repo repo; @@ -26,6 +29,18 @@ public class RepoUpdaterTest { String simpleIndexSigningCert = "308201ee30820157a0030201020204300d845b300d06092a864886f70d01010b0500302a3110300e060355040b1307462d44726f6964311630140603550403130d70616c6174736368696e6b656e301e170d3134303432373030303633315a170d3431303931323030303633315a302a3110300e060355040b1307462d44726f6964311630140603550403130d70616c6174736368696e6b656e30819f300d06092a864886f70d010101050003818d0030818902818100a439472e4b6d01141bfc94ecfe131c7c728fdda670bb14c57ca60bd1c38a8b8bc0879d22a0a2d0bc0d6fdd4cb98d1d607c2caefbe250a0bd0322aedeb365caf9b236992fac13e6675d3184a6c7c6f07f73410209e399a9da8d5d7512bbd870508eebacff8b57c3852457419434d34701ccbf692267cbc3f42f1c5d1e23762d790203010001a321301f301d0603551d0e041604140b1840691dab909746fde4bfe28207d1cae15786300d06092a864886f70d01010b05000381810062424c928ffd1b6fd419b44daafef01ca982e09341f7077fb865905087aeac882534b3bd679b51fdfb98892cef38b63131c567ed26c9d5d9163afc775ac98ad88c405d211d6187bde0b0d236381cc574ba06ef9080721a92ae5a103a7301b2c397eecc141cc850dd3e123813ebc41c59d31ddbcb6e984168280c53272f6a442b"; + /** + * Getting a writeable dir during the tests seems to be a flaky prospect. + */ + private boolean canWrite() { + if (testFilesDir.canWrite()) { + return true; + } else { + Log.e(TAG, "ERROR: " + testFilesDir + " is not writable, skipping test"); + return false; + } + } + @Before public void setUp() { Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); @@ -38,9 +53,7 @@ public class RepoUpdaterTest { @Test public void testExtractIndexFromJar() { - if (!testFilesDir.canWrite()) { - return; - } + assumeTrue(canWrite()); File simpleIndexJar = TestUtils.copyAssetToDir(context, "simpleIndex.jar", testFilesDir); repoUpdater = new RepoUpdater(context, repo); @@ -55,6 +68,7 @@ public class RepoUpdaterTest { @Test(expected = UpdateException.class) public void testExtractIndexFromOutdatedJar() throws UpdateException { + assumeTrue(canWrite()); File simpleIndexJar = TestUtils.copyAssetToDir(context, "simpleIndex.jar", testFilesDir); repo.version = 10; repo.timestamp = System.currentTimeMillis() / 1000L; @@ -67,9 +81,7 @@ public class RepoUpdaterTest { @Test(expected = UpdateException.class) public void testExtractIndexFromJarWithoutSignatureJar() throws UpdateException { - if (!testFilesDir.canWrite()) { - return; - } + assumeTrue(canWrite()); // this is supposed to fail File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithoutSignature.jar", testFilesDir); repoUpdater = new RepoUpdater(context, repo); @@ -79,9 +91,7 @@ public class RepoUpdaterTest { @Test public void testExtractIndexFromJarWithCorruptedManifestJar() { - if (!testFilesDir.canWrite()) { - return; - } + assumeTrue(canWrite()); // this is supposed to fail try { File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedManifest.jar", testFilesDir); @@ -98,9 +108,7 @@ public class RepoUpdaterTest { @Test public void testExtractIndexFromJarWithCorruptedSignature() { - if (!testFilesDir.canWrite()) { - return; - } + assumeTrue(canWrite()); // this is supposed to fail try { File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedSignature.jar", testFilesDir); @@ -117,9 +125,7 @@ public class RepoUpdaterTest { @Test public void testExtractIndexFromJarWithCorruptedCertificate() { - if (!testFilesDir.canWrite()) { - return; - } + assumeTrue(canWrite()); // this is supposed to fail try { File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedCertificate.jar", testFilesDir); @@ -136,9 +142,7 @@ public class RepoUpdaterTest { @Test public void testExtractIndexFromJarWithCorruptedEverything() { - if (!testFilesDir.canWrite()) { - return; - } + assumeTrue(canWrite()); // this is supposed to fail try { File jarFile = TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedEverything.jar", testFilesDir); @@ -155,9 +159,7 @@ public class RepoUpdaterTest { @Test public void testExtractIndexFromMasterKeyIndexJar() { - if (!testFilesDir.canWrite()) { - return; - } + assumeTrue(canWrite()); // this is supposed to fail try { File jarFile = TestUtils.copyAssetToDir(context, "masterKeyIndex.jar", testFilesDir); From 086ff54b5f40eea34ff06d0d952b7099783f8b60 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 20 May 2016 12:19:17 +0200 Subject: [PATCH 5/5] move versionCode to app/build.gradle to match versionName on @mvdan's request --- app/build.gradle | 1 + app/src/main/AndroidManifest.xml | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index d668052a5..3c35d5c87 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -160,6 +160,7 @@ android { } defaultConfig { + versionCode 100007 versionName getVersionName() testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner" } diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 7978e0807..6188c4e87 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -1,8 +1,7 @@ + android:installLocation="auto">