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

Commit 80e1fb9e authored by Sarp Misoglu's avatar Sarp Misoglu
Browse files

Disallow changing backup activation state for private profile

Private profile is used for the 'private space' feature and backup is always disabled for that user. Not even the system server or root user can enable backup for it.

Test: atest BackupManagerServiceTest
Flag: EXEMPT bugfix
Bug: 404533205
Change-Id: I8235f1a0dbdb23d9e7ce3d40b624f9d3e85ce70f
parent 7d29380a
Loading
Loading
Loading
Loading
+13 −7
Original line number Diff line number Diff line
@@ -42,6 +42,7 @@ import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.content.pm.PackageManager;
import android.content.pm.UserInfo;
import android.os.Binder;
import android.os.FileUtils;
import android.os.Handler;
@@ -470,14 +471,19 @@ public class BackupManagerService extends IBackupManager.Stub implements BackupM
    }

    /**
     * The system user and managed profiles can only be acted on by callers in the system or root
     * processes. Other users can be acted on by callers who have both android.permission.BACKUP and
     * Private profiles' activation status is never allowed to be changed by anyone. The system
     * user and managed profiles can only be acted on by callers in the system or root processes.
     * Other users can be acted on by callers who have both android.permission.BACKUP and
     * android.permission.INTERACT_ACROSS_USERS_FULL permissions.
     */
    private void enforcePermissionsOnUser(@UserIdInt int userId) throws SecurityException {
        boolean isRestrictedUser =
                userId == UserHandle.USER_SYSTEM
                        || mUserManagerInternal.getUserInfo(userId).isManagedProfile();
    private void enforceSetBackupServiceActiveAllowedForUser(@UserIdInt int userId)
            throws SecurityException {
        UserInfo userInfo = mUserManagerInternal.getUserInfo(userId);
        if (userInfo.isPrivateProfile()) {
            throw new SecurityException("Changing private profile backup activation not allowed");
        }

        boolean isRestrictedUser = userId == UserHandle.USER_SYSTEM || userInfo.isManagedProfile();

        if (isRestrictedUser) {
            int caller = binderGetCallingUid();
@@ -499,7 +505,7 @@ public class BackupManagerService extends IBackupManager.Stub implements BackupM
     * is unlocked at this point yet, so handle both cases.
     */
    public void setBackupServiceActive(@UserIdInt int userId, boolean makeActive) {
        enforcePermissionsOnUser(userId);
        enforceSetBackupServiceActiveAllowedForUser(userId);

        // In Q, backup is OFF by default for non-system users. In the future, we will change that
        // to ON unless backup was explicitly deactivated with a (permissioned) call to
+2 −0
Original line number Diff line number Diff line
@@ -118,8 +118,10 @@ public class BackupManagerServiceRoboTest {
        LocalServices.removeServiceForTest(UserManagerInternal.class);
        LocalServices.addService(UserManagerInternal.class, mUserManagerInternal);

        when(mUserManagerInternal.getUserInfo(UserHandle.USER_SYSTEM)).thenReturn(mUserInfoMock);
        when(mUserManagerInternal.getUserInfo(mUserOneId)).thenReturn(mUserInfoMock);
        when(mUserManagerInternal.getUserInfo(mUserTwoId)).thenReturn(mUserInfoMock);
        when(mUserInfoMock.isManagedProfile()).thenReturn(false);

        when(mUserOneService.getBackupAgentConnectionManager()).thenReturn(
                mUserOneBackupAgentConnectionManager);
+45 −27
Original line number Diff line number Diff line
@@ -25,8 +25,8 @@ import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertNotNull;
import static junit.framework.Assert.assertNull;
import static junit.framework.Assert.assertTrue;
import static junit.framework.Assert.fail;

import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
@@ -97,7 +97,8 @@ public class BackupManagerServiceTest {
    private static final int NON_SYSTEM_USER = UserHandle.USER_SYSTEM + 1;
    private static final int NON_SYSTEM_NON_DEFAULT_USER = NON_SYSTEM_USER + 1;

    @Rule public final Expect expect = Expect.create();
    @Rule
    public final Expect expect = Expect.create();

    @Rule
    public final SetFlagsRule flags =
@@ -330,6 +331,21 @@ public class BackupManagerServiceTest {
        assertTrue(mService.isBackupServiceActive(NON_SYSTEM_USER));
    }

    @Test
    public void setBackupServiceActive_forPrivateProfile_throws() {
        createBackupManagerServiceAndUnlockSystemUser();
        simulateUserUnlocked(NON_SYSTEM_USER);
        when(mUserInfoMock.isPrivateProfile()).thenReturn(true);
        BackupManagerServiceTestable.sCallingUid = Process.SYSTEM_UID;

        assertThat(
                assertThrows(
                        SecurityException.class,
                        () -> mService.setBackupServiceActive(NON_SYSTEM_USER,
                                true))).hasMessageThat().isEqualTo(
                "Changing private profile backup activation not allowed");
    }

    @Test
    public void setBackupServiceActive_forSystemUserAndCallerSystemUid_createsService() {
        createBackupManagerServiceAndUnlockSystemUser();
@@ -355,11 +371,13 @@ public class BackupManagerServiceTest {
        createBackupManagerServiceAndUnlockSystemUser();
        BackupManagerServiceTestable.sCallingUid = Process.FIRST_APPLICATION_UID;

        try {
            mService.setBackupServiceActive(UserHandle.USER_SYSTEM, true);
            fail();
        } catch (SecurityException expected) {
        }

        assertThat(
                assertThrows(
                        SecurityException.class,
                        () -> mService.setBackupServiceActive(UserHandle.USER_SYSTEM,
                                true))).hasMessageThat().isEqualTo(
                "No permission to configure backup activity");
    }

    @Test
@@ -392,11 +410,12 @@ public class BackupManagerServiceTest {
        when(mUserInfoMock.isManagedProfile()).thenReturn(true);
        BackupManagerServiceTestable.sCallingUid = Process.FIRST_APPLICATION_UID;

        try {
            mService.setBackupServiceActive(NON_SYSTEM_USER, true);
            fail();
        } catch (SecurityException expected) {
        }
        assertThat(
                assertThrows(
                        SecurityException.class,
                        () -> mService.setBackupServiceActive(NON_SYSTEM_USER,
                                true))).hasMessageThat().isEqualTo(
                "No permission to configure backup activity");
    }

    @Test
@@ -406,11 +425,10 @@ public class BackupManagerServiceTest {
                .when(mContextMock)
                .enforceCallingOrSelfPermission(eq(Manifest.permission.BACKUP), anyString());

        try {
            mService.setBackupServiceActive(NON_SYSTEM_USER, true);
            fail();
        } catch (SecurityException expected) {
        }
        assertThrows(
                SecurityException.class,
                () -> mService.setBackupServiceActive(NON_SYSTEM_USER,
                        true));
    }

    @Test
@@ -421,11 +439,10 @@ public class BackupManagerServiceTest {
                .enforceCallingOrSelfPermission(
                        eq(Manifest.permission.INTERACT_ACROSS_USERS_FULL), anyString());

        try {
            mService.setBackupServiceActive(NON_SYSTEM_USER, true);
            fail();
        } catch (SecurityException expected) {
        }
        assertThrows(
                SecurityException.class,
                () -> mService.setBackupServiceActive(NON_SYSTEM_USER,
                        true));
    }

    @Test
@@ -642,7 +659,8 @@ public class BackupManagerServiceTest {
        ISelectBackupTransportCallback.Stub listener =
                new ISelectBackupTransportCallback.Stub() {
                    @Override
                    public void onSuccess(String transportName) {}
                    public void onSuccess(String transportName) {
                    }

                    @Override
                    public void onFailure(int reason) throws RemoteException {