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

Commit 15ee8bf2 authored by Artyom Chen's avatar Artyom Chen Committed by Android (Google) Code Review
Browse files

Merge "Fix isMountDisallowed to check permissions on the correct user" into main

parents e1a973b3 31dd0d53
Loading
Loading
Loading
Loading
+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);
    }

+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"/>
+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())