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

verify APKs on install, and purge ApkDownloader

Browse Source 
This moves the last piece of code to the DownloaderService model. Moving
the APK prep and verification to Installer.installPackage() makes it hard
to mess up and make bugs like installing from the External Storage issue
found by the Cure53 audit.
This commit is contained in:
Hans-Christoph Steiner 2016-04-12 12:31:30 -04:00
parent d07c8b5d74
commit ee37f26c4d
7 changed files with 113 additions and 249 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
app/src/main/java/org/fdroid/fdroid/AppDetails.java
View File

@ -89,7 +89,6 @@ import org.fdroid.fdroid.data.RepoProvider;
import org.fdroid.fdroid.installer.Installer;
import org.fdroid.fdroid.installer.Installer.InstallFailedException;
import org.fdroid.fdroid.installer.Installer.InstallerCallback;
import org.fdroid.fdroid.net.ApkDownloader;
import org.fdroid.fdroid.net.Downloader;
import org.fdroid.fdroid.net.DownloaderService;
@ -304,7 +303,7 @@ public class AppDetails extends AppCompatActivity {
private App app;
private PackageManager packageManager;
private ApkDownloader downloadHandler;
private String activeDownloadUrlString;
private LocalBroadcastManager localBroadcastManager;
private boolean startingIgnoreAll;
@ -324,17 +323,15 @@ public class AppDetails extends AppCompatActivity {
*/
private static class ConfigurationChangeHelper {
public final ApkDownloader downloader;
public final String urlString;
public final App app;
ConfigurationChangeHelper(ApkDownloader downloader, App app) {
this.downloader = downloader;
ConfigurationChangeHelper(String urlString, App app) {
this.urlString = urlString;
this.app = app;
}
}
private boolean inProcessOfChangingConfiguration;
/**
* Attempt to extract the packageName from the intent which launched this activity.
* @return May return null, if we couldn't find the packageName. This should
@ -375,8 +372,8 @@ public class AppDetails extends AppCompatActivity {
ConfigurationChangeHelper previousData = (ConfigurationChangeHelper) getLastCustomNonConfigurationInstance();
if (previousData != null) {
Utils.debugLog(TAG, "Recreating view after configuration change.");
downloadHandler = previousData.downloader;
if (downloadHandler != null) {
activeDownloadUrlString = previousData.urlString;
if (activeDownloadUrlString != null) {
Utils.debugLog(TAG, "Download was in progress before the configuration change, so we will start to listen to its events again.");
}
app = previousData.app;
@ -438,10 +435,9 @@ public class AppDetails extends AppCompatActivity {
* Remove progress listener, suppress progress bar, set downloadHandler to null.
*/
private void cleanUpFinishedDownload() {
if (downloadHandler != null) {
activeDownloadUrlString = null;
headerFragment.removeProgress();
downloadHandler = null;
}
unregisterDownloaderReceivers();
}
protected void onStop() {
@ -458,7 +454,6 @@ public class AppDetails extends AppCompatActivity {
setIgnoreUpdates(app.packageName, app.ignoreAllUpdates, app.ignoreThisUpdate);
}
unregisterDownloaderReceivers();
headerFragment.removeProgress();
}
private void unregisterDownloaderReceivers() {
@ -469,8 +464,8 @@ public class AppDetails extends AppCompatActivity {
}
private void registerDownloaderReceivers() {
if (downloadHandler != null) { // if a download is active
String url = downloadHandler.urlString;
if (activeDownloadUrlString != null) { // if a download is active
String url = activeDownloadUrlString;
localBroadcastManager.registerReceiver(startedReceiver,
DownloaderService.getIntentFilter(url, Downloader.ACTION_STARTED));
localBroadcastManager.registerReceiver(progressReceiver,
@ -550,17 +545,12 @@ public class AppDetails extends AppCompatActivity {
@Override
public Object onRetainCustomNonConfigurationInstance() {
inProcessOfChangingConfiguration = true;
return new ConfigurationChangeHelper(downloadHandler, app);
return new ConfigurationChangeHelper(activeDownloadUrlString, app);
}
@Override
protected void onDestroy() {
if (downloadHandler != null && !inProcessOfChangingConfiguration) {
DownloaderService.cancel(context, downloadHandler.urlString);
cleanUpFinishedDownload();
}
inProcessOfChangingConfiguration = false;
super.onDestroy();
}
@ -853,10 +843,11 @@ public class AppDetails extends AppCompatActivity {
}
private void startDownload(Apk apk, String repoAddress) {
downloadHandler = new ApkDownloader(getBaseContext(), app, apk, repoAddress);
String urlString = Utils.getApkUrl(repoAddress, apk);
activeDownloadUrlString = urlString;
registerDownloaderReceivers();
downloadHandler.download();
headerFragment.startProgress();
DownloaderService.queue(this, activeDownloadUrlString);
}
public void removeApk(String packageName) {
@ -1466,14 +1457,11 @@ public class AppDetails extends AppCompatActivity {
@Override
public void onClick(View view) {
AppDetails appDetails = (AppDetails) getActivity();
if (appDetails == null || appDetails.downloadHandler == null) {
if (appDetails == null || appDetails.activeDownloadUrlString == null) {
return;
}
DownloaderService.cancel(getContext(), appDetails.downloadHandler.urlString);
appDetails.cleanUpFinishedDownload();
setProgressVisible(false);
updateViews();
DownloaderService.cancel(getContext(), appDetails.activeDownloadUrlString);
}
public void updateViews() {
@ -1485,7 +1473,7 @@ public class AppDetails extends AppCompatActivity {
TextView statusView = (TextView) view.findViewById(R.id.status);
btMain.setVisibility(View.VISIBLE);
if (appDetails.downloadHandler != null) {
if (appDetails.activeDownloadUrlString != null) {
btMain.setText(R.string.downloading);
btMain.setEnabled(false);
} else if (!app.isInstalled() && app.suggestedVercode > 0 &&

1
app/src/main/java/org/fdroid/fdroid/FDroidApp.java
View File

@ -234,7 +234,6 @@ public class FDroidApp extends Application {
// been installed, but this causes problems for proprietary gapps
// users since the introduction of verification (on pre-4.2 Android),
// because the install intent says it's finished when it hasn't.
Utils.deleteFiles(Utils.getApkDownloadDir(this), null, ".apk");
if (!Preferences.get().shouldCacheApks()) {
Utils.deleteFiles(Utils.getApkCacheDir(this), null, ".apk");
}

24
app/src/main/java/org/fdroid/fdroid/Utils.java
View File

@ -318,7 +318,11 @@ public final class Utils {
}
/**
* See {@link Utils#getApkDownloadDir(android.content.Context)} for why this is "unsafe".
* This location is only for caching, do not install directly from this location
* because if the file is on the External Storage, any other app could swap out
* the APK while the install was in process, allowing malware to install things.
* Using {@link org.fdroid.fdroid.installer.Installer#installPackage(File, String)}
* is fine since that does the right thing.
*/
public static SanitizedFile getApkCacheDir(Context context) {
final SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, true), "apks");
@ -328,24 +332,6 @@ public final class Utils {
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 getApkDownloadDir(Context context) {
final SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, false), "temp");
if (!apkCacheDir.exists()) {
apkCacheDir.mkdir();
}
// All parent directories of the .apk file need to be executable for the package installer
// to be able to have permission to read our world-readable .apk files.
FileCompat.setExecutable(apkCacheDir, true, false);
return apkCacheDir;
}
public static String calcFingerprint(String keyHexString) {
if (TextUtils.isEmpty(keyHexString)
|| keyHexString.matches(".*[^a-fA-F0-9].*")) {

97
app/src/main/java/org/fdroid/fdroid/installer/Installer.java
View File

@ -23,14 +23,25 @@ import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.text.TextUtils;
import android.util.Log;
import org.apache.commons.io.FileUtils;
import org.fdroid.fdroid.AndroidXMLDecompress;
import org.fdroid.fdroid.BuildConfig;
import org.fdroid.fdroid.Hasher;
import org.fdroid.fdroid.Preferences;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.compat.FileCompat;
import org.fdroid.fdroid.data.Apk;
import org.fdroid.fdroid.data.ApkProvider;
import org.fdroid.fdroid.data.SanitizedFile;
import org.fdroid.fdroid.privileged.install.InstallExtensionDialogActivity;
import java.io.File;
import java.io.IOException;
import java.security.NoSuchAlgorithmException;
import java.util.Map;
/**
* Abstract Installer class. Also provides static methods to automatically
@ -120,16 +131,16 @@ public abstract class Installer {
if (android.os.Build.VERSION.SDK_INT >= 14) {
// Default installer on Android >= 4.0
try {
Utils.debugLog(TAG, "try default installer for Android >= 4");
Utils.debugLog(TAG, "try default installer for android >= 14");
return new DefaultSdk14Installer(activity, pm, callback);
} catch (InstallFailedException e) {
Log.e(TAG, "Android not compatible with DefaultInstallerSdk14!", e);
}
} else {
// Default installer on Android < 4.0
// Default installer on Android < 4.0 (android-14)
try {
Utils.debugLog(TAG, "try default installer for Android < 4");
Utils.debugLog(TAG, "try default installer for android < 14");
return new DefaultInstaller(activity, pm, callback);
} catch (InstallFailedException e) {
@ -141,12 +152,55 @@ public abstract class Installer {
return null;
}
public void installPackage(File apkFile, String packageName) throws InstallFailedException {
// check if file exists...
/**
* Checks the APK file against the provided hash, returning whether it is a match.
*/
private static boolean verifyApkFile(File apkFile, String hash, String hashType)
throws NoSuchAlgorithmException {
if (!apkFile.exists()) {
Log.e(TAG, "Couldn't find file " + apkFile + " to install.");
return;
return false;
}
Hasher hasher = new Hasher(hashType, apkFile);
if (hasher != null && hasher.match(hash)) {
return true;
}
return false;
}
/**
* This is the safe, single point of entry for submitting an APK file to be installed.
*/
public void installPackage(File apkFile, String packageName) throws InstallFailedException {
SanitizedFile apkToInstall = null;
try {
Map<String, Object> attributes = AndroidXMLDecompress.getManifestHeaderAttributes(apkFile.getAbsolutePath());
/* This isn't really needed, but might as well since we have the data already */
if (attributes.containsKey("packageName")) {
if (!TextUtils.equals(packageName, (String) attributes.get("packageName"))) {
throw new InstallFailedException(apkFile + " has packageName that clashes with " + packageName);
}
}
if (!attributes.containsKey("versionCode")) {
throw new InstallFailedException(apkFile + " is missing versionCode!");
}
int versionCode = (Integer) attributes.get("versionCode");
Apk apk = ApkProvider.Helper.find(mContext, packageName, versionCode, new String[]{
ApkProvider.DataColumns.HASH,
ApkProvider.DataColumns.HASH_TYPE,
});
/* Always copy the APK to the safe location inside of the protected area
* of the app to prevent attacks based on other apps swapping the file
* out during the install process. Most likely, apkFile was just downloaded,
* so it should still be in the RAM disk cache */
apkToInstall = SanitizedFile.knownSanitized(File.createTempFile("install-", ".apk", mContext.getFilesDir()));
FileUtils.copyFile(apkFile, apkToInstall);
if (!verifyApkFile(apkToInstall, apk.hash, apk.hashType)) {
FileUtils.deleteQuietly(apkFile);
throw new InstallFailedException(apkFile + " failed to verify!");
}
apkFile = null; // ensure this is not used now that its copied to apkToInstall
// special case: F-Droid Privileged Extension
if (packageName != null && packageName.equals(PrivilegedInstaller.PRIVILEGED_EXTENSION_PACKAGE_NAME)) {
@ -154,26 +208,31 @@ public abstract class Installer {
// extension must be signed with the same public key as main F-Droid
// NOTE: Disabled for debug builds to be able to use official extension from repo
ApkSignatureVerifier signatureVerifier = new ApkSignatureVerifier(mContext);
if (!BuildConfig.DEBUG && !signatureVerifier.hasFDroidSignature(apkFile)) {
throw new SecurityException("APK signature of extension not correct!");
}
Activity activity;
try {
activity = (Activity) mContext;
} catch (ClassCastException e) {
Utils.debugLog(TAG, "F-Droid Privileged can only be updated using an activity!");
return;
if (!BuildConfig.DEBUG && !signatureVerifier.hasFDroidSignature(apkToInstall)) {
throw new InstallFailedException("APK signature of extension not correct!");
}
Activity activity = (Activity) mContext;
Intent installIntent = new Intent(activity, InstallExtensionDialogActivity.class);
installIntent.setAction(InstallExtensionDialogActivity.ACTION_INSTALL);
installIntent.putExtra(InstallExtensionDialogActivity.EXTRA_INSTALL_APK, apkFile.getAbsolutePath());
installIntent.putExtra(InstallExtensionDialogActivity.EXTRA_INSTALL_APK, apkToInstall.getAbsolutePath());
activity.startActivity(installIntent);
return;
}
installPackageInternal(apkFile);
// 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(apkToInstall, true, false);
installPackageInternal(apkToInstall);
} catch (NumberFormatException | NoSuchAlgorithmException | IOException e) {
throw new InstallFailedException(e);
} catch (ClassCastException e) {
throw new InstallFailedException("F-Droid Privileged can only be updated using an activity!");
}
}
public void deletePackage(String packageName) throws InstallFailedException {

160
app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java
View File

@ -1,160 +0,0 @@
/*
* Copyright (C) 2010-2012 Ciaran Gultnieks <ciaran@ciarang.com>
* Copyright (C) 2011 Henrik Tunedal <tunedal@gmail.com>
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 3
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
* MA 02110-1301, USA.
*/
package org.fdroid.fdroid.net;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import android.net.Uri;
import android.support.annotation.NonNull;
import android.support.v4.content.LocalBroadcastManager;
import android.util.Log;
import android.widget.Toast;
import org.fdroid.fdroid.Hasher;
import org.fdroid.fdroid.Preferences;
import org.fdroid.fdroid.R;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.data.Apk;
import org.fdroid.fdroid.data.App;
import org.fdroid.fdroid.data.SanitizedFile;
import java.io.File;
import java.security.NoSuchAlgorithmException;
/**
* Downloads and verifies (against the Apk.hash) the apk file.
* If the file has previously been downloaded, it will make use of that
* instead, without going to the network to download a new one.
*/
public class ApkDownloader {
private static final String TAG = "ApkDownloader";
public final String urlString;
@NonNull private final Apk curApk;
@NonNull private final Context context;
@NonNull private SanitizedFile localFile;
@NonNull private final SanitizedFile potentiallyCachedFile;
private final LocalBroadcastManager localBroadcastManager;
public ApkDownloader(@NonNull final Context context, @NonNull final App app, @NonNull final Apk apk, @NonNull final String repoAddress) {
this.context = context;
curApk = apk;
potentiallyCachedFile = new SanitizedFile(Utils.getApkCacheDir(context), apk.apkName);
urlString = Utils.getApkUrl(repoAddress, apk);
localBroadcastManager = LocalBroadcastManager.getInstance(context);
}
private Hasher createHasher(File apkFile) {
Hasher hasher;
try {
hasher = new Hasher(curApk.hashType, apkFile);
} catch (NoSuchAlgorithmException e) {
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(@NonNull final File apkFile) {
if (!apkFile.exists()) {
return false;
}
Hasher hasher = createHasher(apkFile);
return hasher != null && hasher.match(curApk.hash);
}
/**
* If an existing cached version exists, and matches the hash of the apk we
* 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 verifyOrDelete(@NonNull final File apkFile) {
if (apkFile.exists()) {
if (hashMatches(apkFile)) {
Utils.debugLog(TAG, "Using cached apk at " + apkFile);
return true;
}
Utils.debugLog(TAG, "Not using cached apk at " + apkFile + "(hash doesn't match, will delete file)");
delete(apkFile);
}
return false;
}
private void delete(@NonNull final File file) {
if (file.exists()) {
if (!file.delete()) {
Log.w(TAG, "Could not delete file " + file);
}
}
}
private void sendDownloadComplete() {
Utils.debugLog(TAG, "Download finished: " + localFile);
localBroadcastManager.unregisterReceiver(downloadCompleteReceiver);
}
public void download() {
// Can we use the cached version?
if (verifyOrDelete(potentiallyCachedFile)) {
delete(localFile);
Utils.copyQuietly(potentiallyCachedFile, localFile);
sendDownloadComplete();
return;
}
Utils.debugLog(TAG, "Downloading apk from " + urlString + " to " + localFile);
localBroadcastManager.registerReceiver(downloadCompleteReceiver,
DownloaderService.getIntentFilter(urlString, Downloader.ACTION_COMPLETE));
DownloaderService.queue(context, urlString);
}
private void sendProgressEvent(String status) {
Intent intent = new Intent(status);
intent.setData(Uri.parse(urlString));
intent.putExtra(Downloader.EXTRA_DOWNLOAD_PATH, localFile.getAbsolutePath());
localBroadcastManager.sendBroadcast(intent);
}
// TODO move this code to somewhere more appropriate
BroadcastReceiver downloadCompleteReceiver = new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
localFile = SanitizedFile.knownSanitized(intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH));
if (!verifyOrDelete(localFile)) {
sendProgressEvent(Downloader.ACTION_INTERRUPTED);
Toast.makeText(context, R.string.corrupt_download, Toast.LENGTH_LONG).show();
return;
}
if (Preferences.get().shouldCacheApks()) {
Utils.debugLog(TAG, "Copying .apk file to cache at " + potentiallyCachedFile.getAbsolutePath());
Utils.copyQuietly(localFile, potentiallyCachedFile);
}
sendDownloadComplete();
}
};
}

7
app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java
View File

@ -34,7 +34,6 @@ import android.text.TextUtils;
import android.util.Log;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.compat.FileCompat;
import org.fdroid.fdroid.data.SanitizedFile;
import java.io.File;
@ -194,12 +193,6 @@ public class DownloaderService extends Service {
}
});
downloader.download();
// 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);
sendBroadcast(uri, Downloader.ACTION_COMPLETE, localFile);
} catch (InterruptedException e) {
sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile);

9
app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java
View File

@ -45,7 +45,6 @@ import org.fdroid.fdroid.installer.Installer;
import org.fdroid.fdroid.localrepo.LocalRepoManager;
import org.fdroid.fdroid.localrepo.SwapService;
import org.fdroid.fdroid.localrepo.peers.Peer;
import org.fdroid.fdroid.net.ApkDownloader;
import org.fdroid.fdroid.net.Downloader;
import org.fdroid.fdroid.net.DownloaderService;
@ -783,8 +782,8 @@ public class SwapWorkflowActivity extends AppCompatActivity {
}
public void install(@NonNull final App app) {
final Apk apkToInstall = ApkProvider.Helper.find(this, app.packageName, app.suggestedVercode);
final ApkDownloader downloader = new ApkDownloader(this, app, apkToInstall, apkToInstall.repoAddress);
final Apk apk = ApkProvider.Helper.find(this, app.packageName, app.suggestedVercode);
String urlString = Utils.getApkUrl(apk.repoAddress, apk);
downloadCompleteReceiver = new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
@ -793,8 +792,8 @@ public class SwapWorkflowActivity extends AppCompatActivity {
}
};
localBroadcastManager.registerReceiver(downloadCompleteReceiver,
DownloaderService.getIntentFilter(downloader.urlString, Downloader.ACTION_COMPLETE));
downloader.download();
DownloaderService.getIntentFilter(urlString, Downloader.ACTION_COMPLETE));
DownloaderService.queue(this, urlString);
}
private void handleDownloadComplete(File apkFile, String packageName) {