Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b1613982 authored by Sudheer Shanka's avatar Sudheer Shanka
Browse files

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
parent 24d193cc
Loading
Loading
Loading
Loading
+8 −0
Original line number Diff line number Diff line
@@ -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);
}
+61 −3
Original line number Diff line number Diff line
@@ -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,8 +3243,23 @@ class StorageManagerService extends IStorageManager.Stub
        public void opChanged(int op, int uid, String packageName) throws RemoteException {
            if (!ENABLE_ISOLATED_STORAGE) return;

            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));
            }
        }
    };

    private void addObbStateLocked(ObbState obbState) throws RemoteException {
@@ -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);
                }
            }
        }
    }
}
+13 −0
Original line number Diff line number Diff line
@@ -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,