diff --git a/core/java/android/app/GameManager.java b/core/java/android/app/GameManager.java index 8b6570f1241fa..ac1fa1ec68378 100644 --- a/core/java/android/app/GameManager.java +++ b/core/java/android/app/GameManager.java @@ -16,7 +16,9 @@ package android.app; +import android.Manifest; import android.annotation.IntDef; +import android.annotation.RequiresPermission; import android.annotation.SystemService; import android.annotation.UserHandleAware; import android.content.Context; @@ -73,8 +75,8 @@ public final class GameManager { /** * Returns the game mode for the given package. */ - // TODO(b/178111358): Add @RequiresPermission. @UserHandleAware + @RequiresPermission(Manifest.permission.MANAGE_GAME_MODE) public @GameMode int getGameMode(String packageName) { try { return mService.getGameMode(packageName, mContext.getUserId()); @@ -86,8 +88,8 @@ public final class GameManager { /** * Sets the game mode for the given package. */ - // TODO(b/178111358): Add @RequiresPermission. @UserHandleAware + @RequiresPermission(Manifest.permission.MANAGE_GAME_MODE) public void setGameMode(String packageName, @GameMode int gameMode) { try { mService.setGameMode(packageName, gameMode, mContext.getUserId()); diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index 827bf7b70cbc9..cc7251a4b3b05 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -5447,6 +5447,11 @@ + + + diff --git a/services/core/java/com/android/server/app/GameManagerService.java b/services/core/java/com/android/server/app/GameManagerService.java index 3acad49496c5e..76c34678d5898 100644 --- a/services/core/java/com/android/server/app/GameManagerService.java +++ b/services/core/java/com/android/server/app/GameManagerService.java @@ -16,17 +16,23 @@ package com.android.server.app; +import android.Manifest; import android.annotation.NonNull; +import android.annotation.RequiresPermission; +import android.app.ActivityManager; import android.app.GameManager; import android.app.GameManager.GameMode; import android.app.IGameManagerService; import android.content.Context; +import android.content.pm.PackageManager; +import android.os.Binder; import android.os.Environment; import android.os.Handler; import android.os.Looper; import android.os.Message; import android.os.Process; import android.util.ArrayMap; +import android.util.Log; import android.util.Slog; import com.android.internal.annotations.GuardedBy; @@ -81,6 +87,14 @@ public final class GameManagerService extends IGameManagerService.Stub { switch (msg.what) { case WRITE_SETTINGS: { final int userId = (int) msg.obj; + if (userId < 0) { + Slog.wtf(TAG, "Attempt to write settings for invalid user: " + userId); + synchronized (mLock) { + removeMessages(WRITE_SETTINGS, msg.obj); + } + break; + } + Process.setThreadPriority(Process.THREAD_PRIORITY_DEFAULT); synchronized (mLock) { removeMessages(WRITE_SETTINGS, msg.obj); @@ -94,6 +108,15 @@ public final class GameManagerService extends IGameManagerService.Stub { } case REMOVE_SETTINGS: { final int userId = (int) msg.obj; + if (userId < 0) { + Slog.wtf(TAG, "Attempt to write settings for invalid user: " + userId); + synchronized (mLock) { + removeMessages(WRITE_SETTINGS, msg.obj); + removeMessages(REMOVE_SETTINGS, msg.obj); + } + break; + } + synchronized (mLock) { // Since the user was removed, ignore previous write message // and do write here. @@ -146,9 +169,23 @@ public final class GameManagerService extends IGameManagerService.Stub { } } - //TODO(b/178111358) Add proper permission check and multi-user handling + private boolean hasPermission(String permission) { + return mContext.checkCallingOrSelfPermission(permission) + == PackageManager.PERMISSION_GRANTED; + } + @Override + @RequiresPermission(Manifest.permission.MANAGE_GAME_MODE) public @GameMode int getGameMode(String packageName, int userId) { + if (!hasPermission(Manifest.permission.MANAGE_GAME_MODE)) { + Log.w(TAG, String.format("Caller or self does not have permission.MANAGE_GAME_MODE")); + return GameManager.GAME_MODE_UNSUPPORTED; + } + + userId = ActivityManager.handleIncomingUser(Binder.getCallingPid(), + Binder.getCallingUid(), userId, false, true, "getGameMode", + "com.android.server.app.GameManagerService"); + synchronized (mLock) { if (!mSettings.containsKey(userId)) { return GameManager.GAME_MODE_UNSUPPORTED; @@ -158,9 +195,18 @@ public final class GameManagerService extends IGameManagerService.Stub { } } - //TODO(b/178111358) Add proper permission check and multi-user handling @Override + @RequiresPermission(Manifest.permission.MANAGE_GAME_MODE) public void setGameMode(String packageName, @GameMode int gameMode, int userId) { + if (!hasPermission(Manifest.permission.MANAGE_GAME_MODE)) { + Log.w(TAG, String.format("Caller or self does not have permission.MANAGE_GAME_MODE")); + return; + } + + userId = ActivityManager.handleIncomingUser(Binder.getCallingPid(), + Binder.getCallingUid(), userId, false, true, "setGameMode", + "com.android.server.app.GameManagerService"); + synchronized (mLock) { if (!mSettings.containsKey(userId)) { return; diff --git a/services/tests/servicestests/src/com/android/server/app/GameManagerServiceTests.java b/services/tests/servicestests/src/com/android/server/app/GameManagerServiceTests.java index caa46da306fa5..d039a9d6bfe10 100644 --- a/services/tests/servicestests/src/com/android/server/app/GameManagerServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/app/GameManagerServiceTests.java @@ -18,14 +18,23 @@ package com.android.server.app; import static org.junit.Assert.assertEquals; +import android.Manifest; import android.app.GameManager; +import android.content.Context; +import android.content.ContextWrapper; +import android.content.pm.PackageManager; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; + +import java.util.HashMap; +import java.util.function.Supplier; @RunWith(AndroidJUnit4.class) @SmallTest @@ -37,15 +46,88 @@ public class GameManagerServiceTests { private static final int USER_ID_1 = 1001; private static final int USER_ID_2 = 1002; + // Stolen from ConnectivityServiceTest.MockContext + class MockContext extends ContextWrapper { + private static final String TAG = "MockContext"; + + // Map of permission name -> PermissionManager.Permission_{GRANTED|DENIED} constant + private final HashMap mMockedPermissions = new HashMap<>(); + + MockContext(Context base) { + super(base); + } + + /** + * Mock checks for the specified permission, and have them behave as per {@code granted}. + * + *

Passing null reverts to default behavior, which does a real permission check on the + * test package. + * @param granted One of {@link PackageManager#PERMISSION_GRANTED} or + * {@link PackageManager#PERMISSION_DENIED}. + */ + public void setPermission(String permission, Integer granted) { + mMockedPermissions.put(permission, granted); + } + + private int checkMockedPermission(String permission, Supplier ifAbsent) { + final Integer granted = mMockedPermissions.get(permission); + return granted != null ? granted : ifAbsent.get(); + } + + @Override + public int checkPermission(String permission, int pid, int uid) { + return checkMockedPermission( + permission, () -> super.checkPermission(permission, pid, uid)); + } + + @Override + public int checkCallingOrSelfPermission(String permission) { + return checkMockedPermission( + permission, () -> super.checkCallingOrSelfPermission(permission)); + } + + @Override + public void enforceCallingOrSelfPermission(String permission, String message) { + final Integer granted = mMockedPermissions.get(permission); + if (granted == null) { + super.enforceCallingOrSelfPermission(permission, message); + return; + } + + if (!granted.equals(PackageManager.PERMISSION_GRANTED)) { + throw new SecurityException("[Test] permission denied: " + permission); + } + } + } + + @Mock + private MockContext mMockContext; + + @Before + public void setUp() throws Exception { + mMockContext = new MockContext(InstrumentationRegistry.getContext()); + } + + private void mockModifyGameModeGranted() { + mMockContext.setPermission(Manifest.permission.MANAGE_GAME_MODE, + PackageManager.PERMISSION_GRANTED); + } + + private void mockModifyGameModeDenied() { + mMockContext.setPermission(Manifest.permission.MANAGE_GAME_MODE, + PackageManager.PERMISSION_DENIED); + } + /** * By default game mode is not supported. */ @Test public void testGameModeDefaultValue() { - GameManagerService gameManagerService = - new GameManagerService(InstrumentationRegistry.getContext()); + GameManagerService gameManagerService = new GameManagerService(mMockContext); gameManagerService.onUserStarting(USER_ID_1); + mockModifyGameModeGranted(); + assertEquals(GameManager.GAME_MODE_UNSUPPORTED, gameManagerService.getGameMode(PACKAGE_NAME_0, USER_ID_1)); } @@ -55,10 +137,11 @@ public class GameManagerServiceTests { */ @Test public void testDefaultValueForNonexistentUser() { - GameManagerService gameManagerService = - new GameManagerService(InstrumentationRegistry.getContext()); + GameManagerService gameManagerService = new GameManagerService(mMockContext); gameManagerService.onUserStarting(USER_ID_1); + mockModifyGameModeGranted(); + gameManagerService.setGameMode(PACKAGE_NAME_1, GameManager.GAME_MODE_STANDARD, USER_ID_2); assertEquals(GameManager.GAME_MODE_UNSUPPORTED, gameManagerService.getGameMode(PACKAGE_NAME_1, USER_ID_2)); @@ -69,10 +152,11 @@ public class GameManagerServiceTests { */ @Test public void testGameMode() { - GameManagerService gameManagerService = - new GameManagerService(InstrumentationRegistry.getContext()); + GameManagerService gameManagerService = new GameManagerService(mMockContext); gameManagerService.onUserStarting(USER_ID_1); + mockModifyGameModeGranted(); + assertEquals(GameManager.GAME_MODE_UNSUPPORTED, gameManagerService.getGameMode(PACKAGE_NAME_1, USER_ID_1)); gameManagerService.setGameMode(PACKAGE_NAME_1, GameManager.GAME_MODE_STANDARD, USER_ID_1); @@ -83,4 +167,48 @@ public class GameManagerServiceTests { assertEquals(GameManager.GAME_MODE_PERFORMANCE, gameManagerService.getGameMode(PACKAGE_NAME_1, USER_ID_1)); } + + /** + * Test permission.MANAGE_GAME_MODE is checked + */ + @Test + public void testGetGameModePermissionDenied() { + GameManagerService gameManagerService = new GameManagerService(mMockContext); + gameManagerService.onUserStarting(USER_ID_1); + + // Update the game mode so we can read back something valid. + mockModifyGameModeGranted(); + gameManagerService.setGameMode(PACKAGE_NAME_1, GameManager.GAME_MODE_STANDARD, USER_ID_1); + assertEquals(GameManager.GAME_MODE_STANDARD, + gameManagerService.getGameMode(PACKAGE_NAME_1, USER_ID_1)); + + // Deny permission.MANAGE_GAME_MODE and verify we get back GameManager.GAME_MODE_UNSUPPORTED + mockModifyGameModeDenied(); + assertEquals(GameManager.GAME_MODE_UNSUPPORTED, + gameManagerService.getGameMode(PACKAGE_NAME_1, USER_ID_1)); + } + + /** + * Test permission.MANAGE_GAME_MODE is checked + */ + @Test + public void testSetGameModePermissionDenied() { + GameManagerService gameManagerService = new GameManagerService(mMockContext); + gameManagerService.onUserStarting(USER_ID_1); + + // Update the game mode so we can read back something valid. + mockModifyGameModeGranted(); + gameManagerService.setGameMode(PACKAGE_NAME_1, GameManager.GAME_MODE_STANDARD, USER_ID_1); + assertEquals(GameManager.GAME_MODE_STANDARD, + gameManagerService.getGameMode(PACKAGE_NAME_1, USER_ID_1)); + + // Deny permission.MANAGE_GAME_MODE and verify the game mode is not updated. + mockModifyGameModeDenied(); + gameManagerService.setGameMode(PACKAGE_NAME_1, GameManager.GAME_MODE_PERFORMANCE, + USER_ID_1); + + mockModifyGameModeGranted(); + assertEquals(GameManager.GAME_MODE_STANDARD, + gameManagerService.getGameMode(PACKAGE_NAME_1, USER_ID_1)); + } }