From b1613982d04f78f0cd6ab5929cf33ce77b6e4ca9 Mon Sep 17 00:00:00 2001 From: Sudheer Shanka Date: Thu, 16 May 2019 16:55:50 -0700 Subject: [PATCH] Notify StorageManagerService when storage related app ops change. StorageManagerService needs to trigger update of storage mountpoints when an app gets storage access, so update AppOpsService to notify StorageManagerService synchronously when storage related app ops change. Also, when an app gets REQUEST_INSTALL_PACKAGES appop denied, kill the app. Bug: 132466536 Test: manual Test: atest cts/hostsidetests/appsecurity/src/android/appsecurity/cts/ExternalStorageHostTest.java Change-Id: I130dde1bcffea6c96e5d8c173055737850af6151 --- .../os/storage/StorageManagerInternal.java | 8 +++ .../android/server/StorageManagerService.java | 64 ++++++++++++++++++- .../android/server/appop/AppOpsService.java | 13 ++++ 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/core/java/android/os/storage/StorageManagerInternal.java b/core/java/android/os/storage/StorageManagerInternal.java index 942bf94b1db63..14c299d11a945 100644 --- a/core/java/android/os/storage/StorageManagerInternal.java +++ b/core/java/android/os/storage/StorageManagerInternal.java @@ -16,6 +16,7 @@ package android.os.storage; +import android.annotation.Nullable; import android.os.IVold; /** @@ -101,4 +102,11 @@ public abstract class StorageManagerInternal { * @param listener The listener that will be notified on reset events. */ public abstract void addResetListener(ResetListener listener); + + /** + * Notified when any app op changes so that storage mount points can be updated if the app op + * affects them. + */ + public abstract void onAppOpsChanged(int code, int uid, + @Nullable String packageName, int mode); } diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index fc355b7cce2f7..9ebbfa0b0d4c3 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -96,6 +96,7 @@ import android.os.SystemClock; import android.os.SystemProperties; import android.os.UserHandle; import android.os.UserManager; +import android.os.UserManagerInternal; import android.os.storage.DiskInfo; import android.os.storage.IObbActionListener; import android.os.storage.IStorageEventListener; @@ -3242,7 +3243,22 @@ class StorageManagerService extends IStorageManager.Stub public void opChanged(int op, int uid, String packageName) throws RemoteException { if (!ENABLE_ISOLATED_STORAGE) return; - remountUidExternalStorage(uid, getMountMode(uid, packageName)); + if (op == OP_REQUEST_INSTALL_PACKAGES) { + // Only handling the case when the appop is denied. The other cases will be + // handled in the synchronous callback from AppOpsService. + if (packageName != null && mIAppOpsService.checkOperation( + OP_REQUEST_INSTALL_PACKAGES, uid, packageName) != MODE_ALLOWED) { + try { + ActivityManager.getService().killUid( + UserHandle.getAppId(uid), UserHandle.getUserId(uid), + "OP_REQUEST_INSTALL_PACKAGES is denied"); + } catch (RemoteException e) { + // same process - should not happen + } + } + } else { + remountUidExternalStorage(uid, getMountMode(uid, packageName)); + } } }; @@ -3579,6 +3595,12 @@ class StorageManagerService extends IStorageManager.Stub if (Process.isIsolated(uid)) { return Zygote.MOUNT_EXTERNAL_NONE; } + + final String[] packagesForUid = mIPackageManager.getPackagesForUid(uid); + if (packageName == null) { + packageName = packagesForUid[0]; + } + if (mPmInternal.isInstantApp(packageName, UserHandle.getUserId(uid))) { return Zygote.MOUNT_EXTERNAL_DEFAULT; } @@ -3601,8 +3623,18 @@ class StorageManagerService extends IStorageManager.Stub // runtime permission; this is a firm CDD requirement final boolean hasInstall = mIPackageManager.checkUidPermission(INSTALL_PACKAGES, uid) == PERMISSION_GRANTED; - final boolean hasInstallOp = mIAppOpsService.checkOperation(OP_REQUEST_INSTALL_PACKAGES, - uid, packageName) == MODE_ALLOWED; + boolean hasInstallOp = false; + // OP_REQUEST_INSTALL_PACKAGES is granted/denied per package but vold can't + // update mountpoints of a specific package. So, check the appop for all packages + // sharing the uid and allow same level of storage access for all packages even if + // one of the packages has the appop granted. + for (String uidPackageName : packagesForUid) { + if (mIAppOpsService.checkOperation( + OP_REQUEST_INSTALL_PACKAGES, uid, uidPackageName) == MODE_ALLOWED) { + hasInstallOp = true; + break; + } + } if ((hasInstall || hasInstallOp) && hasWrite) { return Zygote.MOUNT_EXTERNAL_WRITE; } @@ -3870,6 +3902,14 @@ class StorageManagerService extends IStorageManager.Stub if (ENABLE_ISOLATED_STORAGE) { return getMountMode(uid, packageName); } + try { + if (packageName == null) { + final String[] packagesForUid = mIPackageManager.getPackagesForUid(uid); + packageName = packagesForUid[0]; + } + } catch (RemoteException e) { + // Should not happen - same process + } // No locking - CopyOnWriteArrayList int mountMode = Integer.MAX_VALUE; for (ExternalStorageMountPolicy policy : mPolicies) { @@ -3918,5 +3958,23 @@ class StorageManagerService extends IStorageManager.Stub } return true; } + + public void onAppOpsChanged(int code, int uid, + @Nullable String packageName, int mode) { + if (mode == MODE_ALLOWED && (code == OP_READ_EXTERNAL_STORAGE + || code == OP_WRITE_EXTERNAL_STORAGE + || code == OP_REQUEST_INSTALL_PACKAGES)) { + final long token = Binder.clearCallingIdentity(); + try { + final UserManagerInternal userManagerInternal = + LocalServices.getService(UserManagerInternal.class); + if (userManagerInternal.isUserInitialized(UserHandle.getUserId(uid))) { + onExternalStoragePolicyChanged(uid, packageName); + } + } finally { + Binder.restoreCallingIdentity(token); + } + } + } } } diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index 448e7738203ad..3bed9c3dc7e10 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -1314,6 +1314,7 @@ public class AppOpsService extends IAppOpsService.Stub { } if (callbackSpecs == null) { + notifyOpChangedSync(code, uid, null, mode); return; } @@ -1335,6 +1336,16 @@ public class AppOpsService extends IAppOpsService.Stub { } } } + + notifyOpChangedSync(code, uid, null, mode); + } + + private void notifyOpChangedSync(int code, int uid, @NonNull String packageName, int mode) { + final StorageManagerInternal storageManagerInternal = + LocalServices.getService(StorageManagerInternal.class); + if (storageManagerInternal != null) { + storageManagerInternal.onAppOpsChanged(code, uid, packageName, mode); + } } /** @@ -1438,6 +1449,8 @@ public class AppOpsService extends IAppOpsService.Stub { AppOpsService::notifyOpChanged, this, repCbs, code, uid, packageName)); } + + notifyOpChangedSync(code, uid, packageName, mode); } private void notifyOpChanged(ArraySet callbacks, int code,