diff options
author | 2023-02-03 17:25:14 +0000 | |
---|---|---|
committer | 2023-04-24 14:13:47 +0000 | |
commit | da26f028aeb54125923284002b24fcd7058d15d8 (patch) | |
tree | bb3716c3bb47197c639f170588243a72862519e3 | |
parent | 661518fc7096ae5e4dc5d508a45857a9ed820c49 (diff) |
Improve `dumpsys backup`
This change refactors `dumpsys backup` code and adds some improvements -
- Removes `isUserReadyForBackup()` check on calling user. This check is not needed, as it prevents a User with INTERACT_ACROSS_USERS_FULL permission, but not Backup activated, to get info about Backup service on other users. This is particularly relevant in case of headless mode, as Backup service isn't running for SYSTEM_USER.
- Adds `--user <userId>` optional parameter for `dumpsys backup`, which will return info about Backup service for the user passed in argument. This is also helpul in case of multiple users on the device.
Bug: 267755698
Test: atest BackupManagerServiceTest
Manual test by running dumpsys backup with optional arguments.
Change-Id: Id7790a501a8befb12a212f66adfc0cd8cc9f9e8b
-rw-r--r-- | services/backup/java/com/android/server/backup/BackupManagerService.java | 84 | ||||
-rw-r--r-- | services/tests/mockingservicestests/src/com/android/server/backup/BackupManagerServiceTest.java | 45 |
2 files changed, 103 insertions, 26 deletions
diff --git a/services/backup/java/com/android/server/backup/BackupManagerService.java b/services/backup/java/com/android/server/backup/BackupManagerService.java index 3a7aa855ea90..978dc56201da 100644 --- a/services/backup/java/com/android/server/backup/BackupManagerService.java +++ b/services/backup/java/com/android/server/backup/BackupManagerService.java @@ -1515,41 +1515,73 @@ public class BackupManagerService extends IBackupManager.Stub { @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"); } /** @@ -1654,7 +1686,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/tests/mockingservicestests/src/com/android/server/backup/BackupManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/backup/BackupManagerServiceTest.java index b203cf640097..36f52515f1ea 100644 --- a/services/tests/mockingservicestests/src/com/android/server/backup/BackupManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/backup/BackupManagerServiceTest.java @@ -63,6 +63,7 @@ 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; @@ -615,6 +616,50 @@ public class BackupManagerServiceTest { verify(mNonSystemUserBackupManagerService, never()).dump(any(), any(), any()); } + @Test + public void testDumpForOneUser_callerDoesNotHaveInteractAcrossUsersFullPermission_ignored() { + mService.setBackupServiceActive(NON_SYSTEM_USER, true); + simulateUserUnlocked(NON_SYSTEM_USER); + + doThrow(new SecurityException()) + .when(mContextMock) + .enforceCallingOrSelfPermission( + eq(Manifest.permission.INTERACT_ACROSS_USERS_FULL), anyString()); + + String[] args = new String[]{"--user", Integer.toString(NON_SYSTEM_USER)}; + Assert.assertThrows(SecurityException.class, + () -> mService.dumpWithoutCheckingPermission(mFileDescriptorStub, mPrintWriterMock, + args)); + + verify(mNonSystemUserBackupManagerService, never()).dump(any(), any(), any()); + } + + @Test + public void + testDumpForOneUser_callerHasInteractAcrossUsersFullPermission_dumpsOnlySpecifiedUser() { + mService.setBackupServiceActive(NON_SYSTEM_USER, true); + simulateUserUnlocked(NON_SYSTEM_USER); + + String[] args = new String[]{"--user", Integer.toString(UserHandle.USER_SYSTEM)}; + mService.dumpWithoutCheckingPermission(mFileDescriptorStub, mPrintWriterMock, args); + + verify(mSystemUserBackupManagerService).dump(any(), any(), any()); + } + + @Test + public void testDumpForAllUsers_callerHasInteractAcrossUsersFullPermission_dumpsAllUsers() { + mService.setBackupServiceActive(NON_SYSTEM_USER, true); + simulateUserUnlocked(NON_SYSTEM_USER); + + String[] args = new String[]{"users"}; + mService.dumpWithoutCheckingPermission(mFileDescriptorStub, mPrintWriterMock, 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()); + } + /** * Test that {@link BackupManagerService#dump(FileDescriptor, PrintWriter, String[])} dumps * system user information before non-system user information. |