From 1aa87ddf763eff73cfc85a56b3dad3f9034dec7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Wed, 1 Jun 2016 01:54:44 +0200 Subject: [PATCH] PrivilegedInstaller: Fix check for granted permissions The onConnected callback of ServiceConnection is always executed on the main looper of the context that is used to create the service binding. Thus the old code resulted in a deadlock and then in a timeout of the Thread.wait() method. The check for permissions is now called inside install and uninstall callbacks, where it works asynchronously. --- .../fdroid/installer/PrivilegedInstaller.java | 49 +++++++------------ .../InstallExtensionDialogActivity.java | 7 --- .../views/fragments/PreferencesFragment.java | 3 -- 3 files changed, 18 insertions(+), 41 deletions(-) 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 c8f3bd699..24a4001ff 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java @@ -26,11 +26,11 @@ import android.content.Intent; import android.content.ServiceConnection; import android.content.pm.PackageManager; import android.net.Uri; -import android.os.Bundle; import android.os.IBinder; import android.os.RemoteException; import android.util.Log; +import org.fdroid.fdroid.R; import org.fdroid.fdroid.privileged.IPrivilegedCallback; import org.fdroid.fdroid.privileged.IPrivilegedService; @@ -69,7 +69,6 @@ public class PrivilegedInstaller extends Installer { public static final int IS_EXTENSION_INSTALLED_NO = 0; public static final int IS_EXTENSION_INSTALLED_YES = 1; public static final int IS_EXTENSION_INSTALLED_SIGNATURE_PROBLEM = 2; - public static final int IS_EXTENSION_INSTALLED_PERMISSIONS_PROBLEM = 3; // From AOSP source code public static final int ACTION_INSTALL_REPLACE_EXISTING = 2; @@ -275,23 +274,8 @@ public class PrivilegedInstaller extends Installer { return IS_EXTENSION_INSTALLED_NO; } - // check if it has the privileged permissions granted - final Object mutex = new Object(); - final Bundle returnBundle = new Bundle(); - ServiceConnection mServiceConnection = new ServiceConnection() { + ServiceConnection serviceConnection = new ServiceConnection() { public void onServiceConnected(ComponentName name, IBinder service) { - IPrivilegedService privService = IPrivilegedService.Stub.asInterface(service); - - try { - boolean hasPermissions = privService.hasPrivilegedPermissions(); - returnBundle.putBoolean("has_permissions", hasPermissions); - } catch (RemoteException e) { - Log.e(TAG, "RemoteException", e); - } - - synchronized (mutex) { - mutex.notify(); - } } public void onServiceDisconnected(ComponentName name) { @@ -300,26 +284,15 @@ public class PrivilegedInstaller extends Installer { Intent serviceIntent = new Intent(PRIVILEGED_EXTENSION_SERVICE_INTENT); serviceIntent.setPackage(PRIVILEGED_EXTENSION_PACKAGE_NAME); + // try to connect to check for signature try { - context.getApplicationContext().bindService(serviceIntent, mServiceConnection, + context.getApplicationContext().bindService(serviceIntent, serviceConnection, Context.BIND_AUTO_CREATE); } catch (SecurityException e) { Log.e(TAG, "IS_EXTENSION_INSTALLED_SIGNATURE_PROBLEM", e); return IS_EXTENSION_INSTALLED_SIGNATURE_PROBLEM; } - synchronized (mutex) { - try { - mutex.wait(3000); - } catch (InterruptedException ignored) { - } - } - - boolean hasPermissions = returnBundle.getBoolean("has_permissions", false); - if (!hasPermissions) { - Log.e(TAG, "IS_EXTENSION_INSTALLED_PERMISSIONS_PROBLEM"); - return IS_EXTENSION_INSTALLED_PERMISSIONS_PROBLEM; - } return IS_EXTENSION_INSTALLED_YES; } @@ -355,6 +328,13 @@ public class PrivilegedInstaller extends Installer { }; try { + boolean hasPermissions = privService.hasPrivilegedPermissions(); + if (!hasPermissions) { + sendBroadcastInstall(uri, originatingUri, ACTION_INSTALL_INTERRUPTED, + context.getString(R.string.system_install_denied_permissions)); + return; + } + privService.installPackage(sanitizedUri, ACTION_INSTALL_REPLACE_EXISTING, null, callback); } catch (RemoteException e) { @@ -396,6 +376,13 @@ public class PrivilegedInstaller extends Installer { }; try { + boolean hasPermissions = privService.hasPrivilegedPermissions(); + if (!hasPermissions) { + sendBroadcastUninstall(packageName, ACTION_UNINSTALL_INTERRUPTED, + context.getString(R.string.system_install_denied_permissions)); + return; + } + privService.deletePackage(packageName, 0, callback); } catch (RemoteException e) { Log.e(TAG, "RemoteException", e); diff --git a/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtensionDialogActivity.java b/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtensionDialogActivity.java index 4034f72c3..02d4cf6be 100644 --- a/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtensionDialogActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtensionDialogActivity.java @@ -111,7 +111,6 @@ public class InstallExtensionDialogActivity extends FragmentActivity { runFirstTime(context); break; - case PrivilegedInstaller.IS_EXTENSION_INSTALLED_PERMISSIONS_PROBLEM: case PrivilegedInstaller.IS_EXTENSION_INSTALLED_SIGNATURE_PROBLEM: default: // do nothing @@ -375,12 +374,6 @@ public class InstallExtensionDialogActivity extends FragmentActivity { "\n\n" + getString(R.string.system_install_denied_signature); result = Activity.RESULT_CANCELED; break; - case PrivilegedInstaller.IS_EXTENSION_INSTALLED_PERMISSIONS_PROBLEM: - title = getString(R.string.system_install_post_fail); - message = getString(R.string.system_install_post_fail_message) + - "\n\n" + getString(R.string.system_install_denied_permissions); - result = Activity.RESULT_CANCELED; - break; default: throw new RuntimeException("unhandled return"); } diff --git a/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java b/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java index 18baad9e1..ff29171f6 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java +++ b/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java @@ -225,9 +225,6 @@ public class PreferencesFragment extends PreferenceFragment case PrivilegedInstaller.IS_EXTENSION_INSTALLED_SIGNATURE_PROBLEM: message = getActivity().getString(R.string.system_install_denied_signature); break; - case PrivilegedInstaller.IS_EXTENSION_INSTALLED_PERMISSIONS_PROBLEM: - message = getActivity().getString(R.string.system_install_denied_permissions); - break; default: throw new RuntimeException("unhandled return"); }