From 72fcc3d2c5858b3bcfacfd54e87f16a3416d902e Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 21 Mar 2018 12:00:51 +0100 Subject: [PATCH 1/3] clean up content:// vs file:// logic in installation process This hopefully makes apparent which pieces are only related to APKs, and which pieces are used for all installable file types (media, OTA ZIPs, etc) ExtensionInstaller only works on < android-20 anyway, so that's self- enforcing in terms of URI scheme: it'll only ever see file:// URIs. --- .../fdroid/installer/ApkFileProvider.java | 42 ++++++++++++------- .../fdroid/installer/DefaultInstaller.java | 8 ---- .../fdroid/installer/ExtensionInstaller.java | 13 +++--- .../fdroid/installer/FileInstaller.java | 6 --- .../fdroid/fdroid/installer/Installer.java | 15 ++----- .../fdroid/installer/PrivilegedInstaller.java | 6 --- 6 files changed, 36 insertions(+), 54 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/installer/ApkFileProvider.java b/app/src/main/java/org/fdroid/fdroid/installer/ApkFileProvider.java index 8e7ffaef5..c308c7f11 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/ApkFileProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/ApkFileProvider.java @@ -25,7 +25,6 @@ import android.content.pm.PackageInfo; import android.net.Uri; import android.os.Build; import android.support.v4.content.FileProvider; - import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.SanitizedFile; @@ -34,7 +33,8 @@ import java.io.File; import java.io.IOException; /** - * This class has helper methods for preparing apks for installation. + * Helper methods for preparing APKs and arbitrary files for installation, + * either locally or for sending via bluetooth. *

* APK handling for installations: * 1. APKs are downloaded into a cache directory that is either created on SD card @@ -56,34 +56,44 @@ public class ApkFileProvider extends FileProvider { } /** - * Copies the APK into private data directory of F-Droid and returns a "file" or "content" Uri - * to be used for installation. + * Copies the APK into private data directory of F-Droid and returns a + * {@code file://} or {@code content://} URI to be used for the + * actual installation process. Only APKs will ever use a {@code content://} + * URI, any other file will always use a {@code file://} URI since F-Droid + * itself handles their whole installation process. */ - public static Uri getSafeUri(Context context, Uri localApkUri, Apk expectedApk, boolean useContentUri) + public static Uri getSafeUri(Context context, Uri localApkUri, Apk expectedApk) throws IOException { File apkFile = new File(localApkUri.getPath()); SanitizedFile tempApkFile = ApkCache.copyApkFromCacheToFiles(context, apkFile, expectedApk); - return getSafeUri(context, tempApkFile, useContentUri); + return getSafeUri(context, tempApkFile, + Build.VERSION.SDK_INT >= 24 && expectedApk.isApk()); } - private static Uri getSafeUri(Context context, SanitizedFile tempApkFile, boolean useContentUri) { + /** + * Return a {@link Uri} for all install processes to install this package + * from. This supports APKs and all other supported files. It also + * supports all installation methods, e.g. default, privileged, etc. + * It can return either a {@code content://} or {@code file://} URI. + *

+ * APKs need to be world readable, so that the Android system installer + * is able to read it. Saving it into external storage to send it to 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. + */ + private static Uri getSafeUri(Context context, SanitizedFile tempFile, boolean useContentUri) { if (useContentUri) { - // return a content Uri using support libs FileProvider - Uri apkUri = getUriForFile(context, AUTHORITY, tempApkFile); + Uri apkUri = getUriForFile(context, AUTHORITY, tempFile); context.grantUriPermission("org.fdroid.fdroid.privileged", apkUri, Intent.FLAG_GRANT_READ_URI_PERMISSION); context.grantUriPermission("com.android.bluetooth", apkUri, Intent.FLAG_GRANT_READ_URI_PERMISSION); return apkUri; } - // 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. - tempApkFile.setReadable(true, false); + tempFile.setReadable(true, false); - return Uri.fromFile(tempApkFile); + return Uri.fromFile(tempFile); } } diff --git a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java index e35ad48ae..222256377 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java @@ -24,8 +24,6 @@ import android.app.PendingIntent; import android.content.Context; import android.content.Intent; import android.net.Uri; -import android.os.Build; - import org.fdroid.fdroid.data.Apk; /** @@ -82,10 +80,4 @@ public class DefaultInstaller extends Installer { protected boolean isUnattended() { return false; } - - @Override - protected boolean supportsContentUri() { - // Android N only supports content Uris - return Build.VERSION.SDK_INT >= 24; - } } diff --git a/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java index 3d0b11623..8b710e00a 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java @@ -24,7 +24,6 @@ import android.app.PendingIntent; import android.content.Context; import android.content.Intent; import android.net.Uri; - import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.privileged.install.InstallExtensionDialogActivity; @@ -33,11 +32,16 @@ import java.io.File; /** * Special Installer that is only useful to install the Privileged Extension apk - * as a privileged app into the system partition of Android. + * as a privileged app into the system partition of Android. It is deprecated + * because it cannot work on Android versions newer than {@code android-20} or so, + * due to increased SELinux enforcement that restricts what even root can do. *

* This is installer requires user interaction and thus install/uninstall directly * return PendingIntents. + * + * @see Chainfire talks Android Lollipop and the future of rooting */ +@Deprecated public class ExtensionInstaller extends Installer { ExtensionInstaller(Context context, Apk apk) { @@ -94,9 +98,4 @@ public class ExtensionInstaller extends Installer { protected boolean isUnattended() { return false; } - - @Override - protected boolean supportsContentUri() { - return false; - } } diff --git a/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java index edb82acd4..9356159aa 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java @@ -85,10 +85,4 @@ public class FileInstaller extends Installer { protected boolean isUnattended() { return false; } - - @Override - protected boolean supportsContentUri() { - return false; - } - } \ No newline at end of file diff --git a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java index e9299c49a..da6809c1a 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -31,7 +31,6 @@ import android.os.PatternMatcher; import android.support.v4.content.LocalBroadcastManager; import android.text.TextUtils; import android.util.Log; - import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; @@ -144,7 +143,7 @@ public abstract class Installer { PackageManager pm = context.getPackageManager(); if (Build.VERSION.SDK_INT >= 24 && ( pm.getInstallerPackageName(apk.packageName).equals("com.android.packageinstaller") - || pm.getInstallerPackageName(apk.packageName).equals("com.google.android.packageinstaller"))) { + || pm.getInstallerPackageName(apk.packageName).equals("com.google.android.packageinstaller"))) { Utils.debugLog(TAG, "Falling back to default installer for uninstall"); Intent intent = new Intent(context, DefaultInstallerActivity.class); intent.setAction(DefaultInstallerActivity.ACTION_UNINSTALL_PACKAGE); @@ -195,7 +194,7 @@ public abstract class Installer { sendBroadcastUninstall(action, pendingIntent, null); } - void sendBroadcastUninstall(String action, PendingIntent pendingIntent, String errorMessage) { + private void sendBroadcastUninstall(String action, PendingIntent pendingIntent, String errorMessage) { Uri uri = Uri.fromParts("package", apk.packageName, null); Intent intent = new Intent(action); @@ -247,7 +246,7 @@ public abstract class Installer { try { // move apk file to private directory for installation and check hash - sanitizedUri = ApkFileProvider.getSafeUri(context, localApkUri, apk, supportsContentUri()); + sanitizedUri = ApkFileProvider.getSafeUri(context, localApkUri, apk); } catch (IOException e) { Log.e(TAG, e.getMessage(), e); sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_INTERRUPTED, e.getMessage()); @@ -270,7 +269,7 @@ public abstract class Installer { Log.e(TAG, e.getMessage(), e); Log.e(TAG, "Falling back to AOSP DefaultInstaller!"); DefaultInstaller defaultInstaller = new DefaultInstaller(context, apk); - // https://code.google.com/p/android/issues/detail?id=205827 + // https://issuetracker.google.com/issues/37091886 if (Build.VERSION.SDK_INT >= 24) { // content scheme for N and above defaultInstaller.installPackageInternal(sanitizedUri, downloadUri); @@ -298,10 +297,4 @@ public abstract class Installer { * uninstall activities, without the system enforcing a user prompt. */ protected abstract boolean isUnattended(); - - /** - * @return true if the Installer supports content Uris and not just file Uris - */ - protected abstract boolean supportsContentUri(); - } diff --git a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java index 389859587..0bc880887 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java @@ -27,7 +27,6 @@ import android.content.Intent; import android.content.ServiceConnection; import android.content.pm.PackageManager; import android.net.Uri; -import android.os.Build; import android.os.IBinder; import android.os.RemoteException; import android.util.Log; @@ -404,9 +403,4 @@ public class PrivilegedInstaller extends Installer { protected boolean isUnattended() { return true; } - - @Override - protected boolean supportsContentUri() { - return Build.VERSION.SDK_INT >= 24; - } } From 35471db83c0765d9ed156e02db0ee035e5457f09 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 21 Mar 2018 12:07:00 +0100 Subject: [PATCH 2/3] always use sanitized URI from ApkFileProvider in install process The previous commit makes this issue a lot easier to see. ApkFileProvider getSafeUri() was already making the right URI for SDK_INT < 24, but then this bit of logic was using the original URI, which didn't work. Installing from the app's cache dir triggered a "Parse Error". The Android default installer API needs file:// URIs from getFiles(). closes #1310 --- .../fdroid/fdroid/installer/Installer.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java index da6809c1a..5001bda04 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -235,17 +235,26 @@ public abstract class Installer { } /** - * Install apk + * Install apk given the URI that points to the local APK file, and the + * download URI to identify which session this belongs to. This first + * moves the APK file to private directory for the installation process + * to read from. Then the hash of the APK is checked against the + * {@link Apk} instance provided when this {@code Installer} object was + * instantiated. The list of permissions in the APK file and the + * {@code Apk} instance are compared, if they do not match, then the user + * is prompted with the system installer dialog, which shows all the + * permissions that the APK is requesting. * * @param localApkUri points to the local copy of the APK to be installed * @param downloadUri serves as the unique ID for all actions related to the * installation of that specific APK + * @see InstallManagerService + * @see ACTION_INSTALL_PACKAGE Fails For Any Possible Uri */ public void installPackage(Uri localApkUri, Uri downloadUri) { Uri sanitizedUri; try { - // move apk file to private directory for installation and check hash sanitizedUri = ApkFileProvider.getSafeUri(context, localApkUri, apk); } catch (IOException e) { Log.e(TAG, e.getMessage(), e); @@ -269,14 +278,7 @@ public abstract class Installer { Log.e(TAG, e.getMessage(), e); Log.e(TAG, "Falling back to AOSP DefaultInstaller!"); DefaultInstaller defaultInstaller = new DefaultInstaller(context, apk); - // https://issuetracker.google.com/issues/37091886 - if (Build.VERSION.SDK_INT >= 24) { - // content scheme for N and above - defaultInstaller.installPackageInternal(sanitizedUri, downloadUri); - } else { - // file scheme for below N - defaultInstaller.installPackageInternal(localApkUri, downloadUri); - } + defaultInstaller.installPackageInternal(sanitizedUri, downloadUri); return; } } From df65905d1928a593ba8855815deefc290f4c15dc Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 21 Mar 2018 12:07:50 +0100 Subject: [PATCH 3/3] only log installer messages on debug builds This is stabilized, so we can tone down the logging. --- .../main/java/org/fdroid/fdroid/installer/Installer.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java index 5001bda04..c60d53883 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -30,7 +30,6 @@ import android.os.Build; import android.os.PatternMatcher; import android.support.v4.content.LocalBroadcastManager; import android.text.TextUtils; -import android.util.Log; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; @@ -257,7 +256,7 @@ public abstract class Installer { try { sanitizedUri = ApkFileProvider.getSafeUri(context, localApkUri, apk); } catch (IOException e) { - Log.e(TAG, e.getMessage(), e); + Utils.debugLog(TAG, e.getMessage(), e); sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_INTERRUPTED, e.getMessage()); return; } @@ -267,7 +266,7 @@ public abstract class Installer { ApkVerifier apkVerifier = new ApkVerifier(context, localApkUri, apk); apkVerifier.verifyApk(); } catch (ApkVerifier.ApkVerificationException e) { - Log.e(TAG, e.getMessage(), e); + Utils.debugLog(TAG, e.getMessage(), e); sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_INTERRUPTED, e.getMessage()); return; } catch (ApkVerifier.ApkPermissionUnequalException e) { @@ -275,8 +274,8 @@ public abstract class Installer { // and an unattended installer is used, a wrong permission screen // has been shown, thus fallback to AOSP DefaultInstaller! if (isUnattended()) { - Log.e(TAG, e.getMessage(), e); - Log.e(TAG, "Falling back to AOSP DefaultInstaller!"); + Utils.debugLog(TAG, e.getMessage(), e); + Utils.debugLog(TAG, "Falling back to AOSP DefaultInstaller!"); DefaultInstaller defaultInstaller = new DefaultInstaller(context, apk); defaultInstaller.installPackageInternal(sanitizedUri, downloadUri); return;