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

Commit 31dd0d53 authored by Artyom Chen's avatar Artyom Chen
Browse files

Fix isMountDisallowed to check permissions on the correct user 

Currently isMountDisallowed checks permission on a calling user, which
is always the system user, since the message handler runs on the
background thread.
This is not a problem on non-HSUM devices, since DO provisioning
happens on the system user and in COPE the permissions are set on the
parent profile, which is also the system user.
On HSUM devices this is a bug, since DO provisioning happens on user10
and COPE's parent profile is user10 as well.

Check permissions on the user which volume is mounted to instead.

Test: Verified both on HSUM and non-HSUM devices that the policy WAI.
Test: atest StorageManagerServiceTest
Flag: EXEMPT bugfix
Bug: b/346334511
Bug: b/403258207
Change-Id: I014e88bf2797c69d5546bcd02fe8f0933a11ff0e
parent 827ce461

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

+6 −5
Original line number Diff line number Diff line
@@ -513,8 +513,8 @@ class StorageManagerService extends IStorageManager.Stub 
    public static final Pattern KNOWN_APP_DIR_PATHS = Pattern.compile(

            "(?i)(^/storage/[^/]+/(?:([0-9]+)/)?Android/(?:data|media|obb|sandbox)/)([^/]+)(/.*)?");





    private WatchedVolumeInfo findVolumeByIdOrThrow(String id) {

    @VisibleForTesting

    protected WatchedVolumeInfo findVolumeByIdOrThrow(String id) {

        synchronized (mLock) {

            final WatchedVolumeInfo vol = mVolumes.get(id);

            if (vol != null) {
@@ -1954,11 +1954,12 @@ class StorageManagerService extends IStorageManager.Stub 
     */

    private boolean isMountDisallowed(WatchedVolumeInfo vol) {

        UserManager userManager = mContext.getSystemService(UserManager.class);

        UserHandle mountUserHandle = UserHandle.of(vol.getMountUserId());



        boolean isUsbRestricted = false;

        if (vol.getDisk() != null && vol.getDisk().isUsb()) {

            isUsbRestricted = userManager.hasUserRestriction(UserManager.DISALLOW_USB_FILE_TRANSFER,

                    Binder.getCallingUserHandle());

                    mountUserHandle);

        }



        boolean isTypeRestricted = false;
@@ -1966,7 +1967,7 @@ class StorageManagerService extends IStorageManager.Stub 
                || vol.getType() == VolumeInfo.TYPE_STUB) {

            isTypeRestricted = userManager

                    .hasUserRestriction(UserManager.DISALLOW_MOUNT_PHYSICAL_MEDIA,

                    Binder.getCallingUserHandle());

                    mountUserHandle);

        }



        return isUsbRestricted || isTypeRestricted;
@@ -2379,10 +2380,10 @@ class StorageManagerService extends IStorageManager.Stub 
        super.mount_enforcePermission();



        final WatchedVolumeInfo vol = findVolumeByIdOrThrow(volId);

        updateVolumeMountIdIfRequired(vol);

        if (isMountDisallowed(vol)) {

            throw new SecurityException("Mounting " + volId + " restricted by policy");

        }

        updateVolumeMountIdIfRequired(vol);

        mount(vol);

    }

services/tests/mockingservicestests/AndroidManifest.xml

+2 −0
Original line number Diff line number Diff line
@@ -37,6 +37,8 @@ 
    <uses-permission android:name="android.permission.SET_ALWAYS_FINISH" />

    <uses-permission android:name="android.permission.MANAGE_USERS" />

    <uses-permission android:name="android.permission.USE_BIOMETRIC_INTERNAL" />

    <uses-permission android:name="android.permission.MOUNT_UNMOUNT_FILESYSTEMS" />

    <uses-permission android:name="android.permission.STORAGE_INTERNAL" />



    <!-- needed by MasterClearReceiverTest to display a system dialog -->

    <uses-permission android:name="android.permission.INTERNAL_SYSTEM_WINDOW"/>

services/tests/mockingservicestests/src/com/android/server/StorageManagerServiceTest.java

+53 −2
Original line number Diff line number Diff line
@@ -23,18 +23,26 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify; 
import static com.google.common.truth.Truth.assertThat;

import static com.google.common.truth.Truth.assertWithMessage;



import static org.junit.Assert.assertThrows;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.doNothing;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.spy;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



import android.app.PropertyInvalidatedCache;

import android.content.Context;

import android.multiuser.Flags;

import android.os.UserManager;

import android.os.storage.DiskInfo;

import android.os.storage.ICeStorageLockEventListener;

import android.os.storage.StorageManagerInternal;

import android.os.storage.VolumeInfo;

import android.platform.test.flag.junit.SetFlagsRule;



import com.android.modules.utils.testing.ExtendedMockitoRule;

import com.android.server.storage.WatchedVolumeInfo;



import org.junit.After;

import org.junit.Before;
@@ -49,8 +57,10 @@ public class StorageManagerServiceTest { 
            .getInstrumentation().getTargetContext();

    private StorageManagerService mStorageManagerService;

    private StorageManagerInternal mStorageManagerInternal;

    private UserManager mUserManager;



    private static final int TEST_USER_ID = 1001;

    private static final int SECOND_TEST_USER_ID = 1002;



    @Rule

    public final ExtendedMockitoRule mExtendedMockitoRule = new ExtendedMockitoRule.Builder(this)
@@ -81,10 +91,11 @@ public class StorageManagerServiceTest { 
        // Called when WatchedUserStates is constructed

        doNothing().when(() -> UserManager.invalidateIsUserUnlockedCache());



        mStorageManagerService = new StorageManagerService(mRealContext);

        mStorageManagerService = spy(new StorageManagerService(mRealContext));

        mStorageManagerInternal = LocalServices.getService(StorageManagerInternal.class);

        assertWithMessage("LocalServices.getService(StorageManagerInternal.class)")

                .that(mStorageManagerInternal).isNotNull();

        mUserManager = mRealContext.getSystemService(UserManager.class);

    }



    @After
@@ -134,6 +145,46 @@ public class StorageManagerServiceTest { 
        assertNumberOfStorageCallbackReceivers(callbackReceiverSize);

    }



    @Test

    public void testMountWithRestrictionFailure() {

        DiskInfo diskInfo = new DiskInfo("diskInfoId", DiskInfo.FLAG_USB);

        VolumeInfo volumeInfo = new VolumeInfo(

                "testVolId", VolumeInfo.TYPE_PUBLIC, diskInfo, "partGuid"

        );

        volumeInfo.mountUserId = TEST_USER_ID;

        WatchedVolumeInfo watchedVolumeInfo = WatchedVolumeInfo.fromVolumeInfo(volumeInfo);

        doReturn(watchedVolumeInfo).when(mStorageManagerService).findVolumeByIdOrThrow(

                "testVolId");

        android.os.UserHandle userHandleForRestriction = android.os.UserHandle.of(TEST_USER_ID);

        when(

                mUserManager.hasUserRestriction(

                        UserManager.DISALLOW_MOUNT_PHYSICAL_MEDIA, userHandleForRestriction))

                .thenReturn(true);



        assertThrows(SecurityException.class,

                () -> mStorageManagerService.mount(watchedVolumeInfo.getId()));

    }



    @Test

    public void testMountWithoutRestrictionSuccess() {

        DiskInfo diskInfo = new DiskInfo("diskInfoId", DiskInfo.FLAG_USB);

        VolumeInfo volumeInfo = new VolumeInfo("testVolId", VolumeInfo.TYPE_PUBLIC, diskInfo,

                "partGuid");

        volumeInfo.mountUserId = TEST_USER_ID;

        WatchedVolumeInfo watchedVolumeInfo = WatchedVolumeInfo.fromVolumeInfo(volumeInfo);

        doReturn(watchedVolumeInfo).when(mStorageManagerService).findVolumeByIdOrThrow(

                "testVolId");

        // Still set the restriction for one user, but mount on a different user.

        android.os.UserHandle userHandleForRestriction = android.os.UserHandle.of(

                SECOND_TEST_USER_ID);

        when(

                mUserManager.hasUserRestriction(

                        UserManager.DISALLOW_MOUNT_PHYSICAL_MEDIA, userHandleForRestriction))

                .thenReturn(true);



        mStorageManagerService.mount(watchedVolumeInfo.getId());

    }



    private void assertNumberOfStorageCallbackReceivers(int callbackReceiverSize) {

        assertThat(mStorageManagerService.getCeStorageEventCallbacks()).isNotNull();

        assertThat(mStorageManagerService.getCeStorageEventCallbacks().size())