diff options
| author | 2019-05-16 16:55:50 -0700 | |
|---|---|---|
| committer | 2019-05-20 10:57:44 -0700 | |
| commit | b1613982d04f78f0cd6ab5929cf33ce77b6e4ca9 (patch) | |
| tree | b49adbfd2736f5bd746e12f281e87c02cdf47b70 | |
| parent | 24d193cc301621ce9866a39effcb74621e2fef67 (diff) | |
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
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 942bf94b1db6..14c299d11a94 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 fc355b7cce2f..9ebbfa0b0d4c 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 448e7738203a..3bed9c3dc7e1 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<ModeCallback> callbacks, int code, |