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

Commit 3ce38499 authored by Zim's avatar Zim Committed by Nandana Dutt
Browse files

Enforce the preserveLegacyExternalStorage manifest attribute 

In Q, a manifest flag requestLegacyExternalStorage was
introduced to grant legacy apps full access to external storage.

This flag was sticky across app updates so an app already installed on
a users device would typically always have the legacy_storage appop
even if it received an update to stop requesting the manifest flag.

With the R scoped storage model, we would like to break this
stickiness for 2 reasons:
1. Increase the impact of the storage restrictions
2. Ease testing. Because of the stickyness it is difficult
for apps targeting R to actually lose their legacy_storage appop

Unfortunately, we have to still provide a path for apps who care about
migirating their data on external storage from unreachable locations
on R devices. The canonical example is an app with 3 versions:
(1) Qcannot_migrate, (2) Qcan_migrate, (3) Rcannot_migrate

An update from: 1 -> 2 -> 3 will be fine because (2) will migrate the
apps data. However, apps can very well update from (1) - (3) thereby
losing access to their data in unreachable storage locations on R devices.

To facilitate this migration, apps (targeting R) can explicitly
request the preserveLegacyExternalStorage manifest flag. This will
ensure that if they get updated on a device with a previous version of
their app with the legacy_storage appop, they'll keep legacy_storage
status. Of course, fresh installs (of target R apps) will not have the
legacy_storage appop even if they have requestLegacyExternalStorage
and preserveLegacyExternalStorage flags.

By default, this new flag will be false, legacy status will *not* be
preserved (the legacy_storage appop will *not* be sticky). But apps
that care about migrating data can set the flag to true and
will preserve whatever legacy status they had on an existing install.

Test: atest RestrictedPermissionsTest
Test: atest RestrictedStoragePermissionSharedUidTest
Bug: 148944140
Change-Id: Ifd3410ed1a60f4c0e8414fce904139b539e13ad8
parent b14fba21

core/java/android/os/storage/StorageManagerInternal.java

+6 −0
Original line number Diff line number Diff line
@@ -118,4 +118,10 @@ public abstract class StorageManagerInternal { 
     * @param userId the userId for which to reset storage

     */

    public abstract void resetUser(int userId);



    /**

     * Returns {@code true} if the immediate last installed version of an app with {@code uid} had

     * legacy storage, {@code false} otherwise.

     */

    public abstract boolean hasLegacyExternalStorage(int uid);

}

services/core/java/com/android/server/StorageManagerService.java

+76 −1
Original line number Diff line number Diff line
@@ -28,6 +28,10 @@ import static android.app.AppOpsManager.OP_MANAGE_EXTERNAL_STORAGE; 
import static android.app.AppOpsManager.OP_READ_EXTERNAL_STORAGE;

import static android.app.AppOpsManager.OP_REQUEST_INSTALL_PACKAGES;

import static android.app.AppOpsManager.OP_WRITE_EXTERNAL_STORAGE;

import static android.content.pm.PackageManager.MATCH_ANY_USER;

import static android.content.pm.PackageManager.MATCH_DIRECT_BOOT_AWARE;

import static android.content.pm.PackageManager.MATCH_DIRECT_BOOT_UNAWARE;

import static android.content.pm.PackageManager.MATCH_UNINSTALLED_PACKAGES;

import static android.content.pm.PackageManager.PERMISSION_GRANTED;

import static android.os.ParcelFileDescriptor.MODE_READ_WRITE;

import static android.os.storage.OnObbStateChangeListener.ERROR_ALREADY_MOUNTED;
@@ -136,6 +140,7 @@ import android.util.Xml; 
import com.android.internal.annotations.GuardedBy;

import com.android.internal.app.IAppOpsCallback;

import com.android.internal.app.IAppOpsService;

import com.android.internal.content.PackageMonitor;

import com.android.internal.os.AppFuseMount;

import com.android.internal.os.BackgroundThread;

import com.android.internal.os.FuseUnavailableMountException;
@@ -149,6 +154,7 @@ import com.android.internal.util.HexDump; 
import com.android.internal.util.IndentingPrintWriter;

import com.android.internal.util.Preconditions;

import com.android.internal.widget.LockPatternUtils;

import com.android.server.SystemService.TargetUser;

import com.android.server.pm.Installer;

import com.android.server.storage.AppFuseBridge;

import com.android.server.storage.StorageSessionController;
@@ -184,6 +190,7 @@ import java.util.Locale; 
import java.util.Map;

import java.util.Map.Entry;

import java.util.Objects;

import java.util.Set;

import java.util.concurrent.CopyOnWriteArrayList;

import java.util.concurrent.CountDownLatch;

import java.util.concurrent.TimeUnit;
@@ -270,6 +277,11 @@ class StorageManagerService extends IStorageManager.Stub 
        public void onStopUser(int userHandle) {

            mStorageManagerService.onStopUser(userHandle);

        }



        @Override

        public void onStartUser(TargetUser user) {

            mStorageManagerService.snapshotAndMonitorLegacyStorageAppOp(user.getUserHandle());

        }

    }



    private static final boolean DEBUG_EVENTS = false;
@@ -578,6 +590,12 @@ class StorageManagerService extends IStorageManager.Stub 


    private final boolean mIsFuseEnabled;



    @GuardedBy("mLock")

    private final Set<Integer> mUidsWithLegacyExternalStorage = new ArraySet<>();

    // Not guarded by lock, always used on the ActivityManager thread

    private final Map<Integer, PackageMonitor> mPackageMonitorsForUser = new ArrayMap<>();





    class ObbState implements IBinder.DeathRecipient {

        public ObbState(String rawPath, String canonicalPath, int callingUid,

                IObbActionListener token, int nonce, String volId) {
@@ -1145,6 +1163,10 @@ class StorageManagerService extends IStorageManager.Stub 
        } catch (Exception e) {

            Slog.wtf(TAG, e);

        }

        PackageMonitor monitor = mPackageMonitorsForUser.remove(userId);

        if (monitor != null) {

            monitor.unregister();

        }

    }



    private boolean supportsBlockCheckpoint() throws RemoteException {
@@ -1836,6 +1858,49 @@ class StorageManagerService extends IStorageManager.Stub 
        }

    }



    private void updateLegacyStorageApps(String packageName, int uid, boolean hasLegacy) {

        synchronized (mLock) {

            if (hasLegacy) {

                Slog.v(TAG, "Package " + packageName + " has legacy storage");

                mUidsWithLegacyExternalStorage.add(uid);

            } else {

                // TODO(b/149391976): Handle shared user id. Check if there's any other

                // installed app with legacy external storage before removing

                Slog.v(TAG, "Package " + packageName + " does not have legacy storage");

                mUidsWithLegacyExternalStorage.remove(uid);

            }

        }

    }



    private void snapshotAndMonitorLegacyStorageAppOp(UserHandle user) {

        int userId = user.getIdentifier();



        // TODO(b/149391976): Use mIAppOpsService.getPackagesForOps instead of iterating below

        // It should improve performance but the AppOps method doesn't return any app here :(

        // This operation currently takes about ~20ms on a freshly flashed device

        for (ApplicationInfo ai : mPmInternal.getInstalledApplications(MATCH_DIRECT_BOOT_AWARE

                        | MATCH_DIRECT_BOOT_UNAWARE | MATCH_UNINSTALLED_PACKAGES | MATCH_ANY_USER,

                        userId, Process.myUid())) {

            try {

                boolean hasLegacy = mIAppOpsService.checkOperation(OP_LEGACY_STORAGE, ai.uid,

                        ai.packageName) == MODE_ALLOWED;

                updateLegacyStorageApps(ai.packageName, ai.uid, hasLegacy);

            } catch (RemoteException e) {

                Slog.e(TAG, "Failed to check legacy op for package " + ai.packageName, e);

            }

        }



        PackageMonitor monitor = new PackageMonitor() {

                @Override

                public void onPackageRemoved(String packageName, int uid) {

                    updateLegacyStorageApps(packageName, uid, false);

                }

            };

        // TODO(b/149391976): Use different handler?

        monitor.register(mContext, user, true, mHandler);

        mPackageMonitorsForUser.put(userId, monitor);

    }



    private static long getLastAccessTime(AppOpsManager manager,

            int uid, String packageName, int[] ops) {

        long maxTime = 0;
@@ -4337,6 +4402,13 @@ class StorageManagerService extends IStorageManager.Stub 
            mHandler.obtainMessage(H_RESET).sendToTarget();

        }



        @Override

        public boolean hasLegacyExternalStorage(int uid) {

            synchronized (mLock) {

                return mUidsWithLegacyExternalStorage.contains(uid);

            }

        }



        public boolean hasExternalStorage(int uid, String packageName) {

            // No need to check for system uid. This avoids a deadlock between

            // PackageManagerService and AppOpsService.
@@ -4382,8 +4454,11 @@ class StorageManagerService extends IStorageManager.Stub 
                            // volumes, USB OTGs that are rarely mounted. The app will get the

                            // external_storage gid on next organic restart.

                            killAppForOpChange(code, uid, packageName);

                            return;

                        }

                        return;

                    case OP_LEGACY_STORAGE:

                        updateLegacyStorageApps(packageName, uid, mode == MODE_ALLOWED);

                        return;

                }

            }

services/core/java/com/android/server/policy/SoftRestrictedPermissionPolicy.java

+15 −2
Original line number Diff line number Diff line
@@ -35,14 +35,17 @@ import android.compat.annotation.EnabledAfter; 
import android.content.Context;

import android.content.pm.ApplicationInfo;

import android.content.pm.PackageManager;

import android.content.pm.PackageManagerInternal;

import android.os.Binder;

import android.os.IBinder;

import android.os.RemoteException;

import android.os.ServiceManager;

import android.os.UserHandle;

import android.os.storage.StorageManagerInternal;

import android.util.Log;



import com.android.internal.compat.IPlatformCompat;

import com.android.server.LocalServices;



/**

 * The behavior of soft restricted permissions is different for each permission. This class collects
@@ -107,11 +110,16 @@ public abstract class SoftRestrictedPermissionPolicy { 
                final boolean isWhiteListed;

                boolean shouldApplyRestriction;

                final boolean hasRequestedLegacyExternalStorage;

                final boolean shouldPreserveLegacyExternalStorage;

                final boolean hasWriteMediaStorageGrantedForUid;

                final boolean isScopedStorageEnabled;



                if (appInfo != null) {

                    PackageManager pm = context.getPackageManager();

                    PackageManagerInternal pmInternal =

                            LocalServices.getService(PackageManagerInternal.class);

                    StorageManagerInternal smInternal =

                            LocalServices.getService(StorageManagerInternal.class);

                    int flags = pm.getPermissionFlags(permission, appInfo.packageName, user);

                    isWhiteListed = (flags & FLAGS_PERMISSION_RESTRICTION_ANY_EXEMPT) != 0;

                    hasRequestedLegacyExternalStorage = hasUidRequestedLegacyExternalStorage(
@@ -123,12 +131,16 @@ public abstract class SoftRestrictedPermissionPolicy { 
                    isScopedStorageEnabled =

                            isChangeEnabledForUid(context, appInfo, user, ENABLE_SCOPED_STORAGE)

                            || isScopedStorageRequired;

                    shouldPreserveLegacyExternalStorage = pmInternal.getPackage(

                            appInfo.packageName).hasPreserveLegacyExternalStorage()

                            && smInternal.hasLegacyExternalStorage(appInfo.uid);

                    shouldApplyRestriction = (flags & FLAG_PERMISSION_APPLY_RESTRICTION) != 0

                            || isScopedStorageRequired;

                            || (isScopedStorageRequired && !shouldPreserveLegacyExternalStorage);

                } else {

                    isWhiteListed = false;

                    shouldApplyRestriction = false;

                    hasRequestedLegacyExternalStorage = false;

                    shouldPreserveLegacyExternalStorage = false;

                    hasWriteMediaStorageGrantedForUid = false;

                    isScopedStorageEnabled = false;

                }
@@ -150,7 +162,8 @@ public abstract class SoftRestrictedPermissionPolicy { 
                    public boolean mayAllowExtraAppOp() {

                        return !shouldApplyRestriction

                                && (hasRequestedLegacyExternalStorage

                                        || hasWriteMediaStorageGrantedForUid);

                                        || hasWriteMediaStorageGrantedForUid

                                        || shouldPreserveLegacyExternalStorage);

                    }

                    @Override

                    public boolean mayDenyExtraAppOpIfGranted() {