commit 95df5ca7fe47ca1be6e6fb6d7fa085c1e53620dd
author Bookatz <bookatz@google.com> Thu May 30 15:58:46 2019 -0700
committer Bookatz <bookatz@google.com> Thu May 30 16:51:09 2019 -0700
tree edd840bf5c067733bb58b357e872492c9eddd06b
parent 4c07595040008ecf162a909bc1eb60b0f66c8978

switchUser checks INTERACT_ACROSS_USERS_FULL

Previously, there were implicit checks for other permissions. Here we
add an explicit check for INTERACT_ACROSS_USERS_FULL, in line with
startUser and stopUser.

We also clean up the code by introducing a checkCallingPermission
method, but this introduces no functional changes.

Fixes: 128601341
Test: Manual confirmation that a test app cannot call switchUser but
that shell can. Same for all changed methods.

Change-Id: I447ee7a7fbf634fee602613a6287fdccf835128a

services/core/java/com/android/server/am/UserController.java

diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java
index 2399d05..8f6ac69 100644
--- a/services/core/java/com/android/server/am/UserController.java
+++ b/services/core/java/com/android/server/am/UserController.java
@@ -599,15 +599,7 @@
 
     int stopUser(final int userId, final boolean force, final IStopUserCallback stopUserCallback,
             KeyEvictedCallback keyEvictedCallback) {
-        if (mInjector.checkCallingPermission(INTERACT_ACROSS_USERS_FULL)
-                != PackageManager.PERMISSION_GRANTED) {
-            String msg = "Permission Denial: switchUser() from pid="
-                    + Binder.getCallingPid()
-                    + ", uid=" + Binder.getCallingUid()
-                    + " requires " + INTERACT_ACROSS_USERS_FULL;
-            Slog.w(TAG, msg);
-            throw new SecurityException(msg);
-        }
+        checkCallingPermission(INTERACT_ACROSS_USERS_FULL, "stopUser");
         if (userId < 0 || userId == UserHandle.USER_SYSTEM) {
             throw new IllegalArgumentException("Can't stop system user " + userId);
         }
@@ -1004,16 +996,8 @@
             final int userId,
             final boolean foreground,
             @Nullable IProgressListener unlockListener) {
-        if (mInjector.checkCallingPermission(INTERACT_ACROSS_USERS_FULL)
-                != PackageManager.PERMISSION_GRANTED) {
-            String msg = "Permission Denial: switchUser() from pid="
-                    + Binder.getCallingPid()
-                    + ", uid=" + Binder.getCallingUid()
-                    + " requires " + INTERACT_ACROSS_USERS_FULL;
-            Slog.w(TAG, msg);
-            throw new SecurityException(msg);
-        }
 
+        checkCallingPermission(INTERACT_ACROSS_USERS_FULL, "startUser");
         Slog.i(TAG, "Starting userid:" + userId + " fg:" + foreground);
 
         final int callingUid = Binder.getCallingUid();
@@ -1220,16 +1204,7 @@
     }
 
     boolean unlockUser(final int userId, byte[] token, byte[] secret, IProgressListener listener) {
-        if (mInjector.checkCallingPermission(INTERACT_ACROSS_USERS_FULL)
-                != PackageManager.PERMISSION_GRANTED) {
-            String msg = "Permission Denial: unlockUser() from pid="
-                    + Binder.getCallingPid()
-                    + ", uid=" + Binder.getCallingUid()
-                    + " requires " + INTERACT_ACROSS_USERS_FULL;
-            Slog.w(TAG, msg);
-            throw new SecurityException(msg);
-        }
-
+        checkCallingPermission(INTERACT_ACROSS_USERS_FULL, "unlockUser");
         final long binderToken = Binder.clearCallingIdentity();
         try {
             return unlockUserCleared(userId, token, secret, listener);
@@ -1313,6 +1288,7 @@
     }
 
     boolean switchUser(final int targetUserId) {
+        checkCallingPermission(INTERACT_ACROSS_USERS_FULL, "switchUser");
         enforceShellRestriction(UserManager.DISALLOW_DEBUGGING_FEATURES, targetUserId);
         int currentUserId = getCurrentUserId();
         UserInfo targetUserInfo = getUserInfo(targetUserId);
@@ -1667,15 +1643,7 @@
 
     void registerUserSwitchObserver(IUserSwitchObserver observer, String name) {
         Preconditions.checkNotNull(name, "Observer name cannot be null");
-        if (mInjector.checkCallingPermission(INTERACT_ACROSS_USERS_FULL)
-                != PackageManager.PERMISSION_GRANTED) {
-            final String msg = "Permission Denial: registerUserSwitchObserver() from pid="
-                    + Binder.getCallingPid()
-                    + ", uid=" + Binder.getCallingUid()
-                    + " requires " + INTERACT_ACROSS_USERS_FULL;
-            Slog.w(TAG, msg);
-            throw new SecurityException(msg);
-        }
+        checkCallingPermission(INTERACT_ACROSS_USERS_FULL, "registerUserSwitchObserver");
         mUserSwitchObservers.register(observer, name);
     }
 
@@ -1922,6 +1890,18 @@
         return mInjector.getUserManager().exists(userId);
     }
 
+    private void checkCallingPermission(String permission, String methodName) {
+        if (mInjector.checkCallingPermission(permission)
+                != PackageManager.PERMISSION_GRANTED) {
+            String msg = "Permission denial: " + methodName
+                    + "() from pid=" + Binder.getCallingPid()
+                    + ", uid=" + Binder.getCallingUid()
+                    + " requires " + permission;
+            Slog.w(TAG, msg);
+            throw new SecurityException(msg);
+        }
+    }
+
     private void enforceShellRestriction(String restriction, int userHandle) {
         if (Binder.getCallingUid() == SHELL_UID) {
             if (userHandle < 0 || hasUserRestriction(restriction, userHandle)) {