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

Commit f9c27b35 authored by Himanshu Gupta's avatar Himanshu Gupta
Browse files

Fixing Storage Volume listing for Cloned User. 

Current implementation only returns the Volumes for the current user.
In case of Cloned user this causes issues as Cloned user uses Media
Provider of User 0, thereby returning only Volume 0, causing problems in
assigning projectIds, and affecting storage calculation.
The projectId assignement happens via
MediaProvider.updateExternalStorageFileQuota.

Bug: 235321217
Test: atest android.appsecurity.cts.StorageHostTest
Change-Id: I617bae9b201fd66d7b62b6cbd1c48b9f276f1924

Fixing Storage Volume(s) Retrieval.

With ag/19901205 shared_profile's volumes were also listed in
StorageManager#updateExternalStorageFileQuotaType.

However, the above API can be called from MediaProvider process,
without MANAGE_EXTERNAL_STORAGE permissions, resulting in
SecurityException("Only File Manager Apps permitted") to be thrown
from StorageManagerService#getVolumeList

This fix allows the exception to be bypassed in case the caller is
Media Store process.

Bug: 235321217
Test: atest android.appsecurity.cts.StorageHostTest
Change-Id: I6835cc4d29f3e9c85731979aaf9ab12a30f6419b

Downgrading API permissions for updateExternalStorage

With ag/19901205 the permissions for this API were increased,
and calling users required 'MANAGE_EXTERNAL_STORAGE' permissions.

There is now a way to add the same functionality without
adding any additional permissions.

Bug: 235321217
Test: atest android.appsecurity.cts.StorageHostTest
Change-Id: I6b166ac191a80c600b527266f53107b8d9c78177

Merged-In: I6b166ac191a80c600b527266f53107b8d9c78177
Change-Id: I475d0782cab9fa1033e16d2b4dc297b00296783b
parent 4d3ec5c8

core/java/android/os/storage/StorageManager.java

+10 −1
Original line number Diff line number Diff line
@@ -2555,6 +2555,8 @@ public class StorageManager { 
     * This API doesn't require any special permissions, though typical implementations

     * will require being called from an SELinux domain that allows setting file attributes

     * related to quota (eg the GID or project ID).

     * If the calling user has MANAGE_EXTERNAL_STORAGE permissions, quota for shared profile's

     * volumes is also updated.

     *

     * The default platform user of this API is the MediaProvider process, which is

     * responsible for managing all of external storage.
@@ -2575,7 +2577,14 @@ public class StorageManager { 
            @QuotaType int quotaType) throws IOException {

        long projectId;

        final String filePath = path.getCanonicalPath();

        final StorageVolume volume = getStorageVolume(path);

        int volFlags = FLAG_REAL_STATE | FLAG_INCLUDE_INVISIBLE;

        // If caller has MANAGE_EXTERNAL_STORAGE permission, results from User Profile(s) are also

        // returned by enabling FLAG_INCLUDE_SHARED_PROFILE.

        if (mContext.checkSelfPermission(MANAGE_EXTERNAL_STORAGE) == PERMISSION_GRANTED) {

            volFlags |= FLAG_INCLUDE_SHARED_PROFILE;

        }

        final StorageVolume[] availableVolumes = getVolumeList(mContext.getUserId(), volFlags);

        final StorageVolume volume = getStorageVolume(availableVolumes, path);

        if (volume == null) {

            Log.w(TAG, "Failed to update quota type for " + filePath);

            return;

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

+14 −9
Original line number Diff line number Diff line
@@ -3813,6 +3813,13 @@ class StorageManagerService extends IStorageManager.Stub 
        final boolean includeSharedProfile =

                (flags & StorageManager.FLAG_INCLUDE_SHARED_PROFILE) != 0;



        // When the caller is the app actually hosting external storage, we

        // should never attempt to augment the actual storage volume state,

        // otherwise we risk confusing it with race conditions as users go

        // through various unlocked states

        final boolean callerIsMediaStore = UserHandle.isSameApp(callingUid,

                mMediaStoreAuthorityAppId);



        // Only Apps with MANAGE_EXTERNAL_STORAGE should call the API with includeSharedProfile

        if (includeSharedProfile) {

            try {
@@ -3825,7 +3832,12 @@ class StorageManagerService extends IStorageManager.Stub 
                // Checking first entry in packagesFromUid is enough as using "sharedUserId"

                // mechanism is rare and discouraged. Also, Apps that share same UID share the same

                // permissions.

                if (!mStorageManagerInternal.hasExternalStorageAccess(callingUid,

                // Allowing Media Provider is an exception, Media Provider process should be allowed

                // to query users across profiles, even without MANAGE_EXTERNAL_STORAGE access.

                // Note that ordinarily Media provider process has the above permission, but if they

                // are revoked, Storage Volume(s) should still be returned.

                if (!callerIsMediaStore

                        && !mStorageManagerInternal.hasExternalStorageAccess(callingUid,

                                packagesFromUid[0])) {

                    throw new SecurityException("Only File Manager Apps permitted");

                }
@@ -3839,13 +3851,6 @@ class StorageManagerService extends IStorageManager.Stub 
        // point

        final boolean systemUserUnlocked = isSystemUnlocked(UserHandle.USER_SYSTEM);



        // When the caller is the app actually hosting external storage, we

        // should never attempt to augment the actual storage volume state,

        // otherwise we risk confusing it with race conditions as users go

        // through various unlocked states

        final boolean callerIsMediaStore = UserHandle.isSameApp(callingUid,

                mMediaStoreAuthorityAppId);



        final boolean userIsDemo;

        final boolean userKeyUnlocked;

        final boolean storagePermission;