Karl/BobStore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'fix-220' into 'master'

Browse Source 
Cache files to SD card again (if preference set).

**NOTE: Queueing here to be merged after next stable**

A previous security fix meant we no longer stored apk files on the
SD card. However, this should still be a feature that people can opt
for if they want, without being insecure. As such the process is now:

 * First download: put in internal storage (to ensure it can't be
   modified before installing)

 * After download: also copy to SD card for caching.

 * On starting F-Droid:

    + Always delete internal storage apks.

    + Only delete other, cached apks if cache preference is false.

To make the code simpler and less prone to bugs, I had to consider
the fact that if people did not have an accessible SD card, then the
path to a cached apk and a "downloaded but transient" apk cannot be
the same. While possible, it means many checks to see if they are
the same, thorough permission management to prevent security issues,
and makes it harder to clear transient apks when F-Droid starts.

See merge request !71
This commit is contained in:
Peter Serwylo 2015-04-28 21:34:05 +00:00
commit 833db3b5ce
5 changed files with 75 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
F-Droid/src/org/fdroid/fdroid/AppDetails.java
View File

@ -872,7 +872,7 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A
}
private void startDownload(Apk apk, String repoAddress) {
downloadHandler = new ApkDownloader(apk, repoAddress, Utils.getApkCacheDir(getBaseContext()));
downloadHandler = new ApkDownloader(getBaseContext(), apk, repoAddress);
downloadHandler.setProgressListener(this);
if (downloadHandler.download()) {
updateProgressDialog();

3
F-Droid/src/org/fdroid/fdroid/FDroidApp.java
View File

@ -184,7 +184,8 @@ public class FDroidApp extends Application {
SharedPreferences prefs = PreferenceManager
.getDefaultSharedPreferences(getBaseContext());
curTheme = Theme.valueOf(prefs.getString(Preferences.PREF_THEME, Preferences.DEFAULT_THEME));
if (!prefs.getBoolean(Preferences.PREF_CACHE_APK, false)) {
Utils.deleteFiles(Utils.getApkDownloadDir(this), null, ".apk");
if (!Preferences.get().shouldCacheApks()) {
Utils.deleteFiles(Utils.getApkCacheDir(this), null, ".apk");
}

5
F-Droid/src/org/fdroid/fdroid/Preferences.java
View File

@ -67,6 +67,7 @@ public class Preferences implements SharedPreferences.OnSharedPreferenceChangeLi
private static final boolean DEFAULT_ROOT_INSTALLER = false;
private static final boolean DEFAULT_SYSTEM_INSTALLER = false;
private static final boolean DEFAULT_LOCAL_REPO_BONJOUR = true;
private static final boolean DEFAULT_CACHE_APK = false;
private static final boolean DEFAULT_LOCAL_REPO_HTTPS = false;
private static final boolean DEFAULT_INCOMP_VER = false;
private static final boolean DEFAULT_EXPERT = false;
@ -113,6 +114,10 @@ public class Preferences implements SharedPreferences.OnSharedPreferenceChangeLi
return preferences.getBoolean(PREF_LOCAL_REPO_BONJOUR, DEFAULT_LOCAL_REPO_BONJOUR);
}
public boolean shouldCacheApks() {
return preferences.getBoolean(PREF_CACHE_APK, DEFAULT_CACHE_APK);
}
public boolean showIncompatibleVersions() {
return preferences.getBoolean(PREF_INCOMP_VER, DEFAULT_INCOMP_VER);
}

15
F-Droid/src/org/fdroid/fdroid/Utils.java
View File

@ -310,14 +310,25 @@ public final class Utils {
return b.build();
}
/**
* See {@link Utils#getApkDownloadDir(android.content.Context)} for why this is "unsafe".
*/
public static SanitizedFile getApkCacheDir(Context context) {
final SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, true), "apks");
if (!apkCacheDir.exists()) {
apkCacheDir.mkdir();
}
return apkCacheDir;
}
/**
* The directory where .apk files are downloaded (and stored - if the relevant property is enabled).
* This must be on internal storage, to prevent other apps with "write external storage" from being
* able to change the .apk file between F-Droid requesting the Package Manger to install, and the
* Package Manager receiving that request.
*/
public static File getApkCacheDir(Context context) {
SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, false), "apks");
public static File getApkDownloadDir(Context context) {
final SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, false), "temp");
if (!apkCacheDir.exists()) {
apkCacheDir.mkdir();
}

88
F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java
View File

@ -20,11 +20,15 @@
package org.fdroid.fdroid.net;
import android.content.Context;
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.util.Log;
import org.fdroid.fdroid.Hasher;
import org.fdroid.fdroid.Preferences;
import org.fdroid.fdroid.ProgressListener;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.compat.FileCompat;
import org.fdroid.fdroid.data.Apk;
import org.fdroid.fdroid.data.SanitizedFile;
@ -58,9 +62,10 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener {
*/
public static final String EVENT_DATA_ERROR_TYPE = "apkDownloadErrorType";
private final Apk curApk;
private final String repoAddress;
private final SanitizedFile localFile;
@NonNull private final Apk curApk;
@NonNull private final String repoAddress;
@NonNull private final SanitizedFile localFile;
@NonNull private final SanitizedFile potentiallyCachedFile;
private ProgressListener listener;
private AsyncDownloadWrapper dlWrapper = null;
@ -68,7 +73,7 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener {
private int totalSize = 0;
private boolean isComplete = false;
private final long id =++downloadIdCounter;
private final long id = ++downloadIdCounter;
public void setProgressListener(ProgressListener listener) {
this.listener = listener;
@ -78,10 +83,11 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener {
setProgressListener(null);
}
public ApkDownloader(Apk apk, String repoAddress, File destDir) {
public ApkDownloader(@NonNull final Context context, @NonNull final Apk apk, @NonNull final String repoAddress) {
curApk = apk;
this.repoAddress = repoAddress;
localFile = new SanitizedFile(destDir, curApk.apkName);
localFile = new SanitizedFile(Utils.getApkDownloadDir(context), apk.apkName);
potentiallyCachedFile = new SanitizedFile(Utils.getApkCacheDir(context), apk.apkName);
}
/**
@ -104,23 +110,23 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener {
return repoAddress + "/" + curApk.apkName.replace(" ", "%20");
}
private Hasher createHasher() {
private Hasher createHasher(File apkFile) {
Hasher hasher;
try {
hasher = new Hasher(curApk.hashType, localFile);
hasher = new Hasher(curApk.hashType, apkFile);
} catch (NoSuchAlgorithmException e) {
Log.e(TAG, "Error verifying hash of cached apk at " + localFile + ". " +
Log.e(TAG, "Error verifying hash of cached apk at " + apkFile + ". " +
"I don't understand what the " + curApk.hashType + " hash algorithm is :(");
hasher = null;
}
return hasher;
}
private boolean hashMatches() {
if (!localFile.exists()) {
private boolean hashMatches(@NonNull final File apkFile) {
if (!apkFile.exists()) {
return false;
}
Hasher hasher = createHasher();
Hasher hasher = createHasher(apkFile);
return hasher != null && hasher.match(curApk.hash);
}
@ -129,25 +135,35 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener {
* want to download, then we will return true. Otherwise, we return false
* (and remove the cached file - if it exists and didn't match the correct hash).
*/
private boolean verifyOrDeleteCachedVersion() {
if (localFile.exists()) {
if (hashMatches()) {
Log.d(TAG, "Using cached apk at " + localFile);
private boolean verifyOrDelete(@NonNull final File apkFile) {
if (apkFile.exists()) {
if (hashMatches(apkFile)) {
Log.d(TAG, "Using cached apk at " + apkFile);
return true;
}
Log.d(TAG, "Not using cached apk at " + localFile);
deleteLocalFile();
Log.d(TAG, "Not using cached apk at " + apkFile + "(hash doesn't match, will delete file)");
delete(apkFile);
}
return false;
}
private void deleteLocalFile() {
if (localFile != null && localFile.exists()) {
localFile.delete();
private void delete(@NonNull final File file) {
if (file.exists()) {
if (!file.delete()) {
Log.w(TAG, "Could not delete file " + file);
}
}
}
private void sendCompleteMessage() {
private void prepareApkFileAndSendCompleteMessage() {
// Need the apk to be world readable, so that the installer is able to read it.
// Note that saving it into external storage for the purpose of letting the installer
// have access is insecure, because apps with permission to write to the external
// storage can overwrite the app between F-Droid asking for it to be installed and
// the installer actually installing it.
FileCompat.setReadable(localFile, true, false);
isComplete = true;
sendMessage(EVENT_APK_DOWNLOAD_COMPLETE);
}
@ -164,13 +180,15 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener {
public boolean download() {
// Can we use the cached version?
if (verifyOrDeleteCachedVersion()) {
sendCompleteMessage();
if (verifyOrDelete(potentiallyCachedFile)) {
delete(localFile);
Utils.copy(potentiallyCachedFile, localFile);
prepareApkFileAndSendCompleteMessage();
return false;
}
String remoteAddress = getRemoteAddress();
Log.d(TAG, "Downloading apk from " + remoteAddress);
Log.d(TAG, "Downloading apk from " + remoteAddress + " to " + localFile);
try {
Downloader downloader = DownloaderFactory.create(remoteAddress, localFile);
@ -228,26 +246,28 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener {
public void onErrorDownloading(String localisedExceptionDetails) {
Log.e(TAG, "Download failed: " + localisedExceptionDetails);
sendError(ERROR_DOWNLOAD_FAILED);
deleteLocalFile();
delete(localFile);
}
private void cacheIfRequired() {
if (Preferences.get().shouldCacheApks()) {
Log.i(TAG, "Copying .apk file to cache at " + potentiallyCachedFile.getAbsolutePath());
Utils.copy(localFile, potentiallyCachedFile);
}
}
@Override
public void onDownloadComplete() {
if (!verifyOrDeleteCachedVersion()) {
if (!verifyOrDelete(localFile)) {
sendError(ERROR_HASH_MISMATCH);
return;
}
// Need the apk to be world readable, so that the installer is able to read it.
// Note that saving it into external storage for the purpose of letting the installer
// have access is insecure, because apps with permission to write to the external
// storage can overwrite the app between F-Droid asking for it to be installed and
// the installer actually installing it.
FileCompat.setReadable(localFile, true, false);
cacheIfRequired();
Log.d(TAG, "Download finished: " + localFile);
sendCompleteMessage();
prepareApkFileAndSendCompleteMessage();
}
@Override