diff options
3 files changed, 146 insertions, 123 deletions
diff --git a/services/backup/java/com/android/server/backup/BackupManagerService.java b/services/backup/java/com/android/server/backup/BackupManagerService.java index ce9cdc2aeab5..0353d5a962c7 100644 --- a/services/backup/java/com/android/server/backup/BackupManagerService.java +++ b/services/backup/java/com/android/server/backup/BackupManagerService.java @@ -1510,46 +1510,74 @@ public class BackupManagerService extends IBackupManager.Stub { if (!DumpUtils.checkDumpAndUsageStatsPermission(mContext, TAG, pw)) { return; } - dumpWithoutCheckingPermission(fd, pw, args); - } - @VisibleForTesting - void dumpWithoutCheckingPermission(FileDescriptor fd, PrintWriter pw, String[] args) { - int userId = binderGetCallingUserId(); - if (!isUserReadyForBackup(userId)) { - pw.println("Inactive"); + int argIndex = 0; + + String op = nextArg(args, argIndex); + argIndex++; + + if ("--help".equals(op)) { + showDumpUsage(pw); return; } - - if (args != null) { - for (String arg : args) { - if ("-h".equals(arg)) { - pw.println("'dumpsys backup' optional arguments:"); - pw.println(" -h : this help text"); - pw.println(" a[gents] : dump information about defined backup agents"); - pw.println(" transportclients : dump information about transport clients"); - pw.println(" transportstats : dump transport statts"); - pw.println(" users : dump the list of users for which backup service " - + "is running"); - return; - } else if ("users".equals(arg.toLowerCase())) { - pw.print(DUMP_RUNNING_USERS_MESSAGE); - for (int i = 0; i < mUserServices.size(); i++) { - pw.print(" " + mUserServices.keyAt(i)); - } - pw.println(); - return; + if ("users".equals(op)) { + pw.print(DUMP_RUNNING_USERS_MESSAGE); + for (int i = 0; i < mUserServices.size(); i++) { + UserBackupManagerService userBackupManagerService = + getServiceForUserIfCallerHasPermission(mUserServices.keyAt(i), + "dump()"); + if (userBackupManagerService != null) { + pw.print(" " + userBackupManagerService.getUserId()); } } + pw.println(); + return; } - - for (int i = 0; i < mUserServices.size(); i++) { + if ("--user".equals(op)) { + String userArg = nextArg(args, argIndex); + argIndex++; + if (userArg == null) { + showDumpUsage(pw); + return; + } + int userId = UserHandle.parseUserArg(userArg); UserBackupManagerService userBackupManagerService = - getServiceForUserIfCallerHasPermission(mUserServices.keyAt(i), "dump()"); + getServiceForUserIfCallerHasPermission(userId, "dump()"); if (userBackupManagerService != null) { userBackupManagerService.dump(fd, pw, args); } + return; } + if (op == null || "agents".startsWith(op) || "transportclients".equals(op) + || "transportstats".equals(op)) { + for (int i = 0; i < mUserServices.size(); i++) { + UserBackupManagerService userBackupManagerService = + getServiceForUserIfCallerHasPermission(mUserServices.keyAt(i), "dump()"); + if (userBackupManagerService != null) { + userBackupManagerService.dump(fd, pw, args); + } + } + return; + } + + showDumpUsage(pw); + } + + private String nextArg(String[] args, int argIndex) { + if (argIndex >= args.length) { + return null; + } + return args[argIndex]; + } + + private static void showDumpUsage(PrintWriter pw) { + pw.println("'dumpsys backup' optional arguments:"); + pw.println(" --help : this help text"); + pw.println(" a[gents] : dump information about defined backup agents"); + pw.println(" transportclients : dump information about transport clients"); + pw.println(" transportstats : dump transport stats"); + pw.println(" users : dump the list of users for which backup service is running"); + pw.println(" --user <userId> : dump information for user userId"); } /** @@ -1661,7 +1689,7 @@ public class BackupManagerService extends IBackupManager.Stub { * @param message A message to include in the exception if it is thrown. */ void enforceCallingPermissionOnUserId(@UserIdInt int userId, String message) { - if (Binder.getCallingUserHandle().getIdentifier() != userId) { + if (binderGetCallingUserId() != userId) { mContext.enforceCallingOrSelfPermission( Manifest.permission.INTERACT_ACROSS_USERS_FULL, message); } diff --git a/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceRoboTest.java b/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceRoboTest.java index 94ee0a871448..a547d0f94ea3 100644 --- a/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceRoboTest.java +++ b/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceRoboTest.java @@ -17,9 +17,7 @@ package com.android.server.backup; import static android.Manifest.permission.BACKUP; -import static android.Manifest.permission.DUMP; import static android.Manifest.permission.INTERACT_ACROSS_USERS_FULL; -import static android.Manifest.permission.PACKAGE_USAGE_STATS; import static com.android.server.backup.testing.TransportData.backupTransport; @@ -73,10 +71,7 @@ import org.robolectric.shadow.api.Shadow; import org.robolectric.shadows.ShadowContextWrapper; import java.io.File; -import java.io.FileDescriptor; import java.io.IOException; -import java.io.PrintWriter; -import java.io.StringWriter; /** Tests for {@link BackupManagerService}. */ @RunWith(RobolectricTestRunner.class) @@ -1461,67 +1456,12 @@ public class BackupManagerServiceRoboTest { // Service tests // --------------------------------------------- - /** Test that the backup service routes methods correctly to the user that requests it. */ - @Test - public void testDump_onRegisteredUser_callsMethodForUser() throws Exception { - grantDumpPermissions(); - BackupManagerService backupManagerService = createSystemRegisteredService(); - File testFile = createTestFile(); - FileDescriptor fileDescriptor = new FileDescriptor(); - PrintWriter printWriter = new PrintWriter(testFile); - String[] args = {"1", "2"}; - ShadowBinder.setCallingUserHandle(UserHandle.of(UserHandle.USER_SYSTEM)); - - backupManagerService.dump(fileDescriptor, printWriter, args); - - verify(mUserSystemService).dump(fileDescriptor, printWriter, args); - } - - /** Test that the backup service does not route methods for non-registered users. */ - @Test - public void testDump_onUnknownUser_doesNotPropagateCall() throws Exception { - grantDumpPermissions(); - BackupManagerService backupManagerService = createService(); - File testFile = createTestFile(); - FileDescriptor fileDescriptor = new FileDescriptor(); - PrintWriter printWriter = new PrintWriter(testFile); - String[] args = {"1", "2"}; - - backupManagerService.dump(fileDescriptor, printWriter, args); - - verify(mUserOneService, never()).dump(fileDescriptor, printWriter, args); - } - - /** Test that 'dumpsys backup users' dumps the list of users registered in backup service*/ - @Test - public void testDump_users_dumpsListOfRegisteredUsers() { - grantDumpPermissions(); - BackupManagerService backupManagerService = createSystemRegisteredService(); - registerUser(backupManagerService, mUserOneId, mUserOneService); - StringWriter out = new StringWriter(); - PrintWriter writer = new PrintWriter(out); - String[] args = {"users"}; - - backupManagerService.dump(null, writer, args); - - writer.flush(); - assertEquals( - String.format("%s %d %d\n", BackupManagerService.DUMP_RUNNING_USERS_MESSAGE, - UserHandle.USER_SYSTEM, mUserOneId), - out.toString()); - } - private File createTestFile() throws IOException { File testFile = new File(mContext.getFilesDir(), "test"); testFile.createNewFile(); return testFile; } - private void grantDumpPermissions() { - mShadowContext.grantPermissions(DUMP); - mShadowContext.grantPermissions(PACKAGE_USAGE_STATS); - } - /** * Test that the backup services throws a {@link SecurityException} if the caller does not have * INTERACT_ACROSS_USERS_FULL permission and passes a different user id. diff --git a/services/tests/mockingservicestests/src/com/android/server/backup/BackupManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/backup/BackupManagerServiceTest.java index b203cf640097..c4a042370de5 100644 --- a/services/tests/mockingservicestests/src/com/android/server/backup/BackupManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/backup/BackupManagerServiceTest.java @@ -38,7 +38,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.Manifest; -import android.annotation.UserIdInt; import android.app.backup.BackupManager; import android.app.backup.ISelectBackupTransportCallback; import android.app.job.JobScheduler; @@ -59,10 +58,12 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.SmallTest; import com.android.dx.mockito.inline.extended.ExtendedMockito; +import com.android.internal.util.DumpUtils; import com.android.server.SystemService; import com.android.server.backup.utils.RandomAccessFileUtils; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -74,6 +75,7 @@ import org.mockito.quality.Strictness; import java.io.File; import java.io.FileDescriptor; import java.io.PrintWriter; +import java.io.Writer; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -85,8 +87,6 @@ public class BackupManagerServiceTest { "class"); private static final int NON_SYSTEM_USER = UserHandle.USER_SYSTEM + 1; - @UserIdInt - private int mUserId; @Mock private UserBackupManagerService mSystemUserBackupManagerService; @Mock @@ -94,8 +94,6 @@ public class BackupManagerServiceTest { @Mock private Context mContextMock; @Mock - private PrintWriter mPrintWriterMock; - @Mock private UserManager mUserManagerMock; @Mock private UserInfo mUserInfoMock; @@ -104,6 +102,7 @@ public class BackupManagerServiceTest { private BackupManagerServiceTestable mService; private BackupManagerService.Lifecycle mServiceLifecycle; + private FakePrintWriter mFakePrintWriter; private static File sTestDir; private MockitoSession mSession; @@ -114,6 +113,7 @@ public class BackupManagerServiceTest { this) .strictness(Strictness.LENIENT) .spyStatic(UserBackupManagerService.class) + .spyStatic(DumpUtils.class) .startMocking(); doReturn(mSystemUserBackupManagerService).when( () -> UserBackupManagerService.createAndInitializeService( @@ -122,8 +122,7 @@ public class BackupManagerServiceTest { () -> UserBackupManagerService.createAndInitializeService(eq(NON_SYSTEM_USER), any(), any(), any())); - mUserId = UserHandle.USER_SYSTEM; - + when(mNonSystemUserBackupManagerService.getUserId()).thenReturn(NON_SYSTEM_USER); when(mUserManagerMock.getUserInfo(UserHandle.USER_SYSTEM)).thenReturn(mUserInfoMock); when(mUserManagerMock.getUserInfo(NON_SYSTEM_USER)).thenReturn(mUserInfoMock); // Null main user means there is no main user on the device. @@ -139,6 +138,8 @@ public class BackupManagerServiceTest { when(mContextMock.getSystemService(Context.JOB_SCHEDULER_SERVICE)) .thenReturn(mock(JobScheduler.class)); + + mFakePrintWriter = new FakePrintWriter(); } @After @@ -552,7 +553,8 @@ public class BackupManagerServiceTest { } }; - mService.selectBackupTransportAsyncForUser(mUserId, TRANSPORT_COMPONENT_NAME, listener); + mService.selectBackupTransportAsyncForUser( + UserHandle.USER_SYSTEM, TRANSPORT_COMPONENT_NAME, listener); assertEquals(BackupManager.ERROR_BACKUP_NOT_ALLOWED, (int) future.get(5, TimeUnit.SECONDS)); } @@ -560,9 +562,10 @@ public class BackupManagerServiceTest { @Test public void selectBackupTransportAsyncForUser_beforeUserUnlockedWithNullListener_doesNotThrow() throws Exception { - createBackupManagerServiceAndUnlockSystemUser(); + mService = new BackupManagerServiceTestable(mContextMock); - mService.selectBackupTransportAsyncForUser(mUserId, TRANSPORT_COMPONENT_NAME, null); + mService.selectBackupTransportAsyncForUser( + UserHandle.USER_SYSTEM, TRANSPORT_COMPONENT_NAME, null); // No crash. } @@ -570,13 +573,11 @@ public class BackupManagerServiceTest { @Test public void selectBackupTransportAsyncForUser_beforeUserUnlockedListenerThrowing_doesNotThrow() throws Exception { - createBackupManagerServiceAndUnlockSystemUser(); - + mService = new BackupManagerServiceTestable(mContextMock); 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 { @@ -584,55 +585,91 @@ public class BackupManagerServiceTest { } }; - mService.selectBackupTransportAsyncForUser(mUserId, TRANSPORT_COMPONENT_NAME, listener); + mService.selectBackupTransportAsyncForUser( + UserHandle.USER_SYSTEM, TRANSPORT_COMPONENT_NAME, listener); // No crash. } @Test - public void dump_callerDoesNotHaveDumpPermission_ignored() { + public void dump_callerDoesNotHaveDumpOrUsageStatsPermission_ignored() { + mockDumpPermissionsGranted(false); createBackupManagerServiceAndUnlockSystemUser(); - when(mContextMock.checkCallingOrSelfPermission( - Manifest.permission.DUMP)).thenReturn( - PackageManager.PERMISSION_DENIED); + when(mContextMock.checkCallingOrSelfPermission(Manifest.permission.DUMP)) + .thenReturn(PackageManager.PERMISSION_DENIED); - mService.dump(mFileDescriptorStub, mPrintWriterMock, new String[0]); + mService.dump(mFileDescriptorStub, mFakePrintWriter, new String[0]); verify(mSystemUserBackupManagerService, never()).dump(any(), any(), any()); verify(mNonSystemUserBackupManagerService, never()).dump(any(), any(), any()); } @Test - public void dump_callerDoesNotHavePackageUsageStatsPermission_ignored() { + public void dump_forOneUser_callerDoesNotHaveInteractAcrossUsersFullPermission_ignored() { + mockDumpPermissionsGranted(true); createBackupManagerServiceAndUnlockSystemUser(); - when(mContextMock.checkCallingOrSelfPermission( - Manifest.permission.PACKAGE_USAGE_STATS)).thenReturn( - PackageManager.PERMISSION_DENIED); + mService.setBackupServiceActive(NON_SYSTEM_USER, true); + simulateUserUnlocked(NON_SYSTEM_USER); + doThrow(new SecurityException()) + .when(mContextMock) + .enforceCallingOrSelfPermission( + eq(Manifest.permission.INTERACT_ACROSS_USERS_FULL), anyString()); - mService.dump(mFileDescriptorStub, mPrintWriterMock, new String[0]); + String[] args = new String[] {"--user", Integer.toString(NON_SYSTEM_USER)}; + Assert.assertThrows( + SecurityException.class, + () -> mService.dump(mFileDescriptorStub, mFakePrintWriter, args)); + verify(mNonSystemUserBackupManagerService, never()).dump(any(), any(), any()); + } + + @Test + public void dump_forOneUser_callerHasInteractAcrossUsersFullPermission_dumpsSpecifiedUser() { + mockDumpPermissionsGranted(true); + createBackupManagerServiceAndUnlockSystemUser(); + mService.setBackupServiceActive(NON_SYSTEM_USER, true); + simulateUserUnlocked(NON_SYSTEM_USER); + + String[] args = new String[] {"--user", Integer.toString(UserHandle.USER_SYSTEM)}; + mService.dump(mFileDescriptorStub, mFakePrintWriter, args); + + verify(mSystemUserBackupManagerService).dump(any(), any(), any()); + } + + @Test + public void dump_users_callerHasInteractAcrossUsersFullPermission_dumpsUsers() { + mockDumpPermissionsGranted(true); + createBackupManagerServiceAndUnlockSystemUser(); + mService.setBackupServiceActive(NON_SYSTEM_USER, true); + simulateUserUnlocked(NON_SYSTEM_USER); + + String[] args = new String[] {"users"}; + mService.dump(mFileDescriptorStub, mFakePrintWriter, args); + + // Check that dump() invocations are not called on user's Backup service, + // as 'dumpsys backup users' only list users for whom Backup service is running. verify(mSystemUserBackupManagerService, never()).dump(any(), any(), any()); verify(mNonSystemUserBackupManagerService, never()).dump(any(), any(), any()); + assertThat(mFakePrintWriter.mPrintedSoFar) + .isEqualTo("Backup Manager is running for users: 0 1"); } - /** - * Test that {@link BackupManagerService#dump(FileDescriptor, PrintWriter, String[])} dumps - * system user information before non-system user information. - */ @Test - public void testDump_systemUserFirst() { + public void dump_allUsers_dumpsSystemUserFirst() { + mockDumpPermissionsGranted(true); createBackupManagerServiceAndUnlockSystemUser(); mService.setBackupServiceActive(NON_SYSTEM_USER, true); simulateUserUnlocked(NON_SYSTEM_USER); + String[] args = new String[0]; - mService.dumpWithoutCheckingPermission(mFileDescriptorStub, mPrintWriterMock, args); + mService.dump(mFileDescriptorStub, mFakePrintWriter, args); InOrder inOrder = inOrder(mSystemUserBackupManagerService, mNonSystemUserBackupManagerService); inOrder.verify(mSystemUserBackupManagerService) - .dump(mFileDescriptorStub, mPrintWriterMock, args); + .dump(mFileDescriptorStub, mFakePrintWriter, args); inOrder.verify(mNonSystemUserBackupManagerService) - .dump(mFileDescriptorStub, mPrintWriterMock, args); + .dump(mFileDescriptorStub, mFakePrintWriter, args); inOrder.verifyNoMoreInteractions(); } @@ -753,6 +790,11 @@ public class BackupManagerServiceTest { return new File(sTestDir, "rememberActivated-" + userId); } + private static void mockDumpPermissionsGranted(boolean granted) { + doReturn(granted) + .when(() -> DumpUtils.checkDumpAndUsageStatsPermission(any(), any(), any())); + } + private static class BackupManagerServiceTestable extends BackupManagerService { static boolean sBackupDisabled = false; static int sCallingUserId = -1; @@ -803,4 +845,17 @@ public class BackupManagerServiceTest { runnable.run(); } } + + private static class FakePrintWriter extends PrintWriter { + String mPrintedSoFar = ""; + + FakePrintWriter() { + super(Writer.nullWriter()); + } + + @Override + public void print(String s) { + mPrintedSoFar = mPrintedSoFar + s; + } + } } |