From f4d8bd16b7788abd26313ec2be3a630b43c233c9 Mon Sep 17 00:00:00 2001 From: Michael Groover Date: Mon, 27 Sep 2021 19:09:49 -0700 Subject: [PATCH 1/6] Ensure pkg uid matches provided uid for device phone number check An app on the device is able to directly interact with any of the services that accepts a package name and can return a protected device resource (phone number or identifier). The app is then able to pass the name of another package targeting pre-R and determine whether the app is installed on the device based on whether the service method throws an Exception or not. While the app is able to pass another package's name to the service method, the service method will still use Binder#getCallingUid for the check. To prevent leaking information about packages installed on the device, this commit adds an additional check to verify the provided uid matches that of the package; if not, a SecurityException is thrown that only contains the provided package name, along with the uid / pid of the calling app. Bug: 193441322 Bug: 193445182 Test: atest LegacyPermissionManagerServiceTest Change-Id: If9353b7cb697bd78ab18775aee7723e984d3c1db --- .../LegacyPermissionManagerService.java | 25 +++++ .../LegacyPermissionManagerServiceTest.java | 103 ++++++++++++++++-- 2 files changed, 118 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/pm/permission/LegacyPermissionManagerService.java b/services/core/java/com/android/server/pm/permission/LegacyPermissionManagerService.java index b1676d0e545ff..ea554d3d79964 100644 --- a/services/core/java/com/android/server/pm/permission/LegacyPermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/LegacyPermissionManagerService.java @@ -30,6 +30,7 @@ import android.os.Process; import android.os.ServiceManager; import android.os.UserHandle; import android.permission.ILegacyPermissionManager; +import android.util.EventLog; import android.util.Log; import com.android.internal.annotations.VisibleForTesting; @@ -187,10 +188,25 @@ public class LegacyPermissionManagerService extends ILegacyPermissionManager.Stu private void verifyCallerCanCheckAccess(String packageName, String message, int pid, int uid) { // If the check is being requested by an app then only allow the app to query its own // access status. + boolean reportError = false; int callingUid = mInjector.getCallingUid(); int callingPid = mInjector.getCallingPid(); if (UserHandle.getAppId(callingUid) >= Process.FIRST_APPLICATION_UID && (callingUid != uid || callingPid != pid)) { + reportError = true; + } + // If the query is against an app on the device, then the check should only be allowed if + // the provided uid matches that of the specified package. + if (packageName != null && UserHandle.getAppId(uid) >= Process.FIRST_APPLICATION_UID) { + int packageUid = mInjector.getPackageUidForUser(packageName, UserHandle.getUserId(uid)); + if (uid != packageUid) { + EventLog.writeEvent(0x534e4554, "193441322", + UserHandle.getAppId(callingUid) >= Process.FIRST_APPLICATION_UID + ? callingUid : uid, "Package uid mismatch"); + reportError = true; + } + } + if (reportError) { String response = String.format( "Calling uid %d, pid %d cannot access for package %s (uid=%d, pid=%d): %s", callingUid, callingPid, packageName, uid, pid, message); @@ -385,12 +401,14 @@ public class LegacyPermissionManagerService extends ILegacyPermissionManager.Stu @VisibleForTesting public static class Injector { private final Context mContext; + private final PackageManagerInternal mPackageManagerInternal; /** * Public constructor that accepts a {@code context} within which to operate. */ public Injector(@NonNull Context context) { mContext = context; + mPackageManagerInternal = LocalServices.getService(PackageManagerInternal.class); } /** @@ -453,5 +471,12 @@ public class LegacyPermissionManagerService extends ILegacyPermissionManager.Stu return mContext.getPackageManager().getApplicationInfoAsUser(packageName, 0, UserHandle.getUserHandleForUid(uid)); } + + /** + * Returns the uid for the specified {@code packageName} under the provided {@code userId}. + */ + public int getPackageUidForUser(String packageName, int userId) { + return mPackageManagerInternal.getPackageUid(packageName, 0, userId); + } } } diff --git a/services/tests/servicestests/src/com/android/server/pm/permission/LegacyPermissionManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/pm/permission/LegacyPermissionManagerServiceTest.java index acd3fcab5e52a..3261dfaa95c93 100644 --- a/services/tests/servicestests/src/com/android/server/pm/permission/LegacyPermissionManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/permission/LegacyPermissionManagerServiceTest.java @@ -125,7 +125,7 @@ public class LegacyPermissionManagerServiceTest { public void checkDeviceIdentifierAccess_hasPrivilegedPermission_returnsGranted() { // Apps with the READ_PRIVILEGED_PHONE_STATE permission should have access to device // identifiers. - setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID); + setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID); when(mInjector.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, APP_PID, APP_UID)).thenReturn(PackageManager.PERMISSION_GRANTED); @@ -140,7 +140,7 @@ public class LegacyPermissionManagerServiceTest { public void checkDeviceIdentifierAccess_hasAppOp_returnsGranted() { // Apps that have been granted the READ_DEVICE_IDENTIFIERS appop should have access to // device identifiers. - setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID); + setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID); when(mAppOpsManager.noteOpNoThrow(eq(AppOpsManager.OPSTR_READ_DEVICE_IDENTIFIERS), eq(APP_UID), eq(mPackageName), any(), any())).thenReturn( AppOpsManager.MODE_ALLOWED); @@ -156,7 +156,7 @@ public class LegacyPermissionManagerServiceTest { public void checkDeviceIdentifierAccess_hasDpmAccess_returnsGranted() { // Apps that pass a DevicePolicyManager device / profile owner check should have access to // device identifiers. - setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID); + setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID); when(mDevicePolicyManager.hasDeviceIdentifierAccess(mPackageName, APP_PID, APP_UID)).thenReturn(true); @@ -236,7 +236,7 @@ public class LegacyPermissionManagerServiceTest { // both the permission and the appop must be granted. If the permission is granted but the // appop is not then AppOpsManager#MODE_IGNORED should be returned to indicate that this // should be a silent failure. - setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID); + setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID); setPackageTargetSdk(Build.VERSION_CODES.Q); grantPermissionAndAppop(android.Manifest.permission.READ_PHONE_STATE, null); @@ -256,7 +256,7 @@ public class LegacyPermissionManagerServiceTest { // Apps targeting R+ with just the READ_PHONE_STATE permission granted should not have // access to the phone number; PERMISSION_DENIED should be returned both with and without // the appop granted since this check should be skipped for target SDK R+. - setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID); + setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID); grantPermissionAndAppop(android.Manifest.permission.READ_PHONE_STATE, null); int resultWithoutAppop = mLegacyPermissionManagerService.checkPhoneNumberAccess( @@ -319,12 +319,79 @@ public class LegacyPermissionManagerServiceTest { assertEquals(PackageManager.PERMISSION_GRANTED, resultWithAppop); } + @Test + public void checkPhoneNumberAccess_providedUidDoesNotMatchPackageUid_throwsException() + throws Exception { + // An app can directly interact with one of the services that accepts a package name and + // returns a protected resource via a direct binder transact. This app could then provide + // the name of another app that targets pre-R, then determine if the app is installed based + // on whether the service throws an exception or not. While the app can provide the package + // name of another app, it cannot specify the package uid which is passed to the + // LegacyPermissionManager using Binder#getCallingUid. Ultimately this uid should then be + // compared against the actual uid of the package to ensure information about packages + // installed on the device is not leaked. + setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID + 1); + + assertThrows(SecurityException.class, + () -> mLegacyPermissionManagerService.checkPhoneNumberAccess(mPackageName, + CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID)); + } + + @Test + public void checkPhoneNumberAccess_nullPackageNameSystemUid_returnsGranted() throws Exception { + // The platform can pass a null package name when checking if the platform itself has + // access to the device phone number(s) / identifier(s). This test ensures if a null package + // is provided, then the package uid check is skipped and the test is based on whether the + // the provided uid / pid has been granted the privileged permission. + setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID, -1); + when(mInjector.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, + SYSTEM_PID, SYSTEM_UID)).thenReturn(PackageManager.PERMISSION_GRANTED); + + int result = mLegacyPermissionManagerService.checkPhoneNumberAccess(null, + CHECK_PHONE_NUMBER_MESSAGE, null, SYSTEM_PID, SYSTEM_UID); + + assertEquals(PackageManager.PERMISSION_GRANTED, result); + } + + @Test + public void checkPhoneNumberAccess_systemUidMismatchPackageUid_returnsGranted() + throws Exception { + // When the platform is checking device phone number / identifier access checks for other + // components on the platform, a uid less than the first application UID is provided; this + // test verifies the package uid check is skipped and access is still granted with the + // privileged permission. + int telephonyUid = SYSTEM_UID + 1; + int telephonyPid = SYSTEM_PID + 1; + setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID, -1); + when(mInjector.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, + telephonyPid, telephonyUid)).thenReturn(PackageManager.PERMISSION_GRANTED); + + int result = mLegacyPermissionManagerService.checkPhoneNumberAccess(mPackageName, + CHECK_PHONE_NUMBER_MESSAGE, null, telephonyPid, telephonyUid); + + assertEquals(PackageManager.PERMISSION_GRANTED, result); + } + /** * Configures device identifier access tests to fail; tests verifying access should individually * set an access check to succeed to verify access when that condition is met. */ private void setupCheckDeviceIdentifierAccessTest(int callingPid, int callingUid) { - setupAccessTest(callingPid, callingUid); + setupCheckDeviceIdentifierAccessTest(callingPid, callingUid, callingUid); + } + + /** + * Configures device identifier access tests to fail; tests verifying access should individually + * set an access check to succeed to verify access when that condition is met. + * + *

To prevent leaking package information, access checks for package UIDs >= {@link + * android.os.Process#FIRST_APPLICATION_UID} must ensure the provided uid matches the uid of + * the package being checked; to ensure this check is successful, this method accepts the + * {@code packageUid} to be used for the package being checked. + */ + public void setupCheckDeviceIdentifierAccessTest(int callingPid, int callingUid, + int packageUid) { + setupAccessTest(callingPid, callingUid, packageUid); when(mDevicePolicyManager.hasDeviceIdentifierAccess(anyString(), anyInt(), anyInt())).thenReturn(false); @@ -333,11 +400,26 @@ public class LegacyPermissionManagerServiceTest { } /** - * Configures phone number access tests to fail; tests verifying access should individually set - * an access check to succeed to verify access when that condition is met. + * Configures phone number access tests to fail; tests verifying access should individually + * set an access check to succeed to verify access when that condition is set. + * */ private void setupCheckPhoneNumberAccessTest(int callingPid, int callingUid) throws Exception { - setupAccessTest(callingPid, callingUid); + setupCheckPhoneNumberAccessTest(callingPid, callingUid, callingUid); + } + + /** + * Configures phone number access tests to fail; tests verifying access should individually set + * an access check to succeed to verify access when that condition is met. + * + *

To prevent leaking package information, access checks for package UIDs >= {@link + * android.os.Process#FIRST_APPLICATION_UID} must ensure the provided uid matches the uid of + * the package being checked; to ensure this check is successful, this method accepts the + * {@code packageUid} to be used for the package being checked. + */ + private void setupCheckPhoneNumberAccessTest(int callingPid, int callingUid, int packageUid) + throws Exception { + setupAccessTest(callingPid, callingUid, packageUid); setPackageTargetSdk(Build.VERSION_CODES.R); } @@ -345,9 +427,10 @@ public class LegacyPermissionManagerServiceTest { * Configures the common mocks for any access tests using the provided {@code callingPid} * and {@code callingUid}. */ - private void setupAccessTest(int callingPid, int callingUid) { + private void setupAccessTest(int callingPid, int callingUid, int packageUid) { when(mInjector.getCallingPid()).thenReturn(callingPid); when(mInjector.getCallingUid()).thenReturn(callingUid); + when(mInjector.getPackageUidForUser(anyString(), anyInt())).thenReturn(packageUid); when(mInjector.checkPermission(anyString(), anyInt(), anyInt())).thenReturn( PackageManager.PERMISSION_DENIED); From 2e4241337d7325de4373383eb5a6fc0b4dbfe460 Mon Sep 17 00:00:00 2001 From: Evan Severson Date: Tue, 5 Oct 2021 09:18:41 -0700 Subject: [PATCH 2/6] Fix setting camera op restriction on reboot In SensorPrivacyService we set the camera state to what was read on the microphone's persisted value. Test: atest SensorPrivacyServiceMockingTest Fixes: 201793410 Change-Id: If5cbd2b6cd2c8e155b8a56e0692a0379d188ffce --- .../android/server/SensorPrivacyService.java | 13 +- .../persisted_file_micMute_camMute.xml | 7 ++ .../persisted_file_micMute_camUnmute.xml | 7 ++ .../persisted_file_micUnmute_camMute.xml | 7 ++ .../persisted_file_micUnmute_camUnmute.xml | 7 ++ .../SensorPrivacyServiceMockingTest.java | 112 +++++++++++++++++- 6 files changed, 146 insertions(+), 7 deletions(-) create mode 100644 services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micMute_camMute.xml create mode 100644 services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micMute_camUnmute.xml create mode 100644 services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micUnmute_camMute.xml create mode 100644 services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micUnmute_camUnmute.xml diff --git a/services/core/java/com/android/server/SensorPrivacyService.java b/services/core/java/com/android/server/SensorPrivacyService.java index 93a820cf14069..06a78c864cac2 100644 --- a/services/core/java/com/android/server/SensorPrivacyService.java +++ b/services/core/java/com/android/server/SensorPrivacyService.java @@ -39,6 +39,7 @@ import static android.hardware.SensorPrivacyManager.Sources.OTHER; import static android.hardware.SensorPrivacyManager.Sources.QS_TILE; import static android.hardware.SensorPrivacyManager.Sources.SETTINGS; import static android.hardware.SensorPrivacyManager.Sources.SHELL; +import static android.os.UserHandle.USER_NULL; import static android.os.UserHandle.USER_SYSTEM; import static android.service.SensorPrivacyIndividualEnabledSensorProto.UNKNOWN; @@ -195,7 +196,7 @@ public final class SensorPrivacyService extends SystemService { private EmergencyCallHelper mEmergencyCallHelper; private KeyguardManager mKeyguardManager; - private int mCurrentUser = -1; + private int mCurrentUser = USER_NULL; public SensorPrivacyService(Context context) { super(context); @@ -228,9 +229,9 @@ public final class SensorPrivacyService extends SystemService { @Override public void onUserStarting(TargetUser user) { - if (mCurrentUser == -1) { + if (mCurrentUser == USER_NULL) { mCurrentUser = user.getUserIdentifier(); - mSensorPrivacyServiceImpl.userSwitching(-1, user.getUserIdentifier()); + mSensorPrivacyServiceImpl.userSwitching(USER_NULL, user.getUserIdentifier()); } } @@ -1294,13 +1295,13 @@ public final class SensorPrivacyService extends SystemService { micState = isIndividualSensorPrivacyEnabledLocked(to, MICROPHONE); camState = isIndividualSensorPrivacyEnabledLocked(to, CAMERA); } - if (prevMicState != micState) { + if (from == USER_NULL || prevMicState != micState) { mHandler.onUserGlobalSensorPrivacyChanged(MICROPHONE, micState); setGlobalRestriction(MICROPHONE, micState); } - if (prevCamState != camState) { + if (from == USER_NULL || prevCamState != camState) { mHandler.onUserGlobalSensorPrivacyChanged(CAMERA, camState); - setGlobalRestriction(CAMERA, micState); + setGlobalRestriction(CAMERA, camState); } } diff --git a/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micMute_camMute.xml b/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micMute_camMute.xml new file mode 100644 index 0000000000000..a4de08a854873 --- /dev/null +++ b/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micMute_camMute.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micMute_camUnmute.xml b/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micMute_camUnmute.xml new file mode 100644 index 0000000000000..47649d7392e64 --- /dev/null +++ b/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micMute_camUnmute.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micUnmute_camMute.xml b/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micUnmute_camMute.xml new file mode 100644 index 0000000000000..4fd9ebf987174 --- /dev/null +++ b/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micUnmute_camMute.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micUnmute_camUnmute.xml b/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micUnmute_camUnmute.xml new file mode 100644 index 0000000000000..e8f9edfde2d7d --- /dev/null +++ b/services/tests/mockingservicestests/assets/SensorPrivacyServiceMockingTest/persisted_file_micUnmute_camUnmute.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/services/tests/mockingservicestests/src/com/android/server/sensorprivacy/SensorPrivacyServiceMockingTest.java b/services/tests/mockingservicestests/src/com/android/server/sensorprivacy/SensorPrivacyServiceMockingTest.java index ba79a764b6729..38f01b5acc0c8 100644 --- a/services/tests/mockingservicestests/src/com/android/server/sensorprivacy/SensorPrivacyServiceMockingTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/sensorprivacy/SensorPrivacyServiceMockingTest.java @@ -16,12 +16,18 @@ package com.android.server.sensorprivacy; +import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyInt; + import android.app.ActivityManager; import android.app.ActivityTaskManager; import android.app.AppOpsManager; +import android.app.AppOpsManagerInternal; import android.content.Context; import android.content.pm.UserInfo; import android.os.Environment; @@ -33,8 +39,10 @@ import androidx.test.platform.app.InstrumentationRegistry; import com.android.dx.mockito.inline.extended.ExtendedMockito; import com.android.server.LocalServices; import com.android.server.SensorPrivacyService; +import com.android.server.SystemService; import com.android.server.pm.UserManagerInternal; +import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -44,6 +52,7 @@ import org.mockito.quality.Strictness; import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.util.concurrent.CompletableFuture; @RunWith(AndroidTestingRunner.class) public class SensorPrivacyServiceMockingTest { @@ -63,10 +72,21 @@ public class SensorPrivacyServiceMockingTest { public static final String PERSISTENCE_FILE6 = String.format(PERSISTENCE_FILE_PATHS_TEMPLATE, 6); + public static final String PERSISTENCE_FILE_MIC_MUTE_CAM_MUTE = + "SensorPrivacyServiceMockingTest/persisted_file_micMute_camMute.xml"; + public static final String PERSISTENCE_FILE_MIC_MUTE_CAM_UNMUTE = + "SensorPrivacyServiceMockingTest/persisted_file_micMute_camUnmute.xml"; + public static final String PERSISTENCE_FILE_MIC_UNMUTE_CAM_MUTE = + "SensorPrivacyServiceMockingTest/persisted_file_micUnmute_camMute.xml"; + public static final String PERSISTENCE_FILE_MIC_UNMUTE_CAM_UNMUTE = + "SensorPrivacyServiceMockingTest/persisted_file_micUnmute_camUnmute.xml"; + private Context mContext; @Mock private AppOpsManager mMockedAppOpsManager; @Mock + private AppOpsManagerInternal mMockedAppOpsManagerInternal; + @Mock private UserManagerInternal mMockedUserManagerInternal; @Mock private ActivityManager mMockedActivityManager; @@ -134,13 +154,103 @@ public class SensorPrivacyServiceMockingTest { } } + @Test + public void testServiceInit_AppOpsRestricted_micMute_camMute() throws IOException { + testServiceInit_AppOpsRestricted(PERSISTENCE_FILE_MIC_MUTE_CAM_MUTE, true, true); + } + + @Test + public void testServiceInit_AppOpsRestricted_micMute_camUnmute() throws IOException { + testServiceInit_AppOpsRestricted(PERSISTENCE_FILE_MIC_MUTE_CAM_UNMUTE, true, false); + } + + @Test + public void testServiceInit_AppOpsRestricted_micUnmute_camMute() throws IOException { + testServiceInit_AppOpsRestricted(PERSISTENCE_FILE_MIC_UNMUTE_CAM_MUTE, false, true); + } + + @Test + public void testServiceInit_AppOpsRestricted_micUnmute_camUnmute() throws IOException { + testServiceInit_AppOpsRestricted(PERSISTENCE_FILE_MIC_UNMUTE_CAM_UNMUTE, false, false); + } + + private void testServiceInit_AppOpsRestricted(String persistenceFileMicMuteCamMute, + boolean expectedMicState, boolean expectedCamState) + throws IOException { + MockitoSession mockitoSession = ExtendedMockito.mockitoSession() + .initMocks(this) + .strictness(Strictness.WARN) + .spyStatic(LocalServices.class) + .spyStatic(Environment.class) + .startMocking(); + + try { + mContext = InstrumentationRegistry.getInstrumentation().getContext(); + spyOn(mContext); + + doReturn(mMockedAppOpsManager).when(mContext).getSystemService(AppOpsManager.class); + doReturn(mMockedAppOpsManagerInternal) + .when(() -> LocalServices.getService(AppOpsManagerInternal.class)); + doReturn(mMockedUserManagerInternal) + .when(() -> LocalServices.getService(UserManagerInternal.class)); + doReturn(mMockedActivityManager).when(mContext).getSystemService(ActivityManager.class); + doReturn(mMockedActivityTaskManager) + .when(mContext).getSystemService(ActivityTaskManager.class); + doReturn(mMockedTelephonyManager).when(mContext).getSystemService( + TelephonyManager.class); + + String dataDir = mContext.getApplicationInfo().dataDir; + doReturn(new File(dataDir)).when(() -> Environment.getDataSystemDirectory()); + + File onDeviceFile = new File(dataDir, "sensor_privacy.xml"); + onDeviceFile.delete(); + + doReturn(new int[]{0}).when(mMockedUserManagerInternal).getUserIds(); + doReturn(ExtendedMockito.mock(UserInfo.class)).when(mMockedUserManagerInternal) + .getUserInfo(0); + + CompletableFuture micState = new CompletableFuture<>(); + CompletableFuture camState = new CompletableFuture<>(); + doAnswer(invocation -> { + int code = invocation.getArgument(0); + boolean restricted = invocation.getArgument(1); + if (code == AppOpsManager.OP_RECORD_AUDIO) { + micState.complete(restricted); + } else if (code == AppOpsManager.OP_CAMERA) { + camState.complete(restricted); + } + return null; + }).when(mMockedAppOpsManagerInternal).setGlobalRestriction(anyInt(), anyBoolean(), + any()); + + initServiceWithPersistenceFile(onDeviceFile, persistenceFileMicMuteCamMute, 0); + + Assert.assertTrue(micState.join() == expectedMicState); + Assert.assertTrue(camState.join() == expectedCamState); + + } finally { + mockitoSession.finishMocking(); + } + } + private void initServiceWithPersistenceFile(File onDeviceFile, String persistenceFilePath) throws IOException { + initServiceWithPersistenceFile(onDeviceFile, persistenceFilePath, -1); + } + + private void initServiceWithPersistenceFile(File onDeviceFile, + String persistenceFilePath, int startingUserId) throws IOException { if (persistenceFilePath != null) { Files.copy(mContext.getAssets().open(persistenceFilePath), onDeviceFile.toPath()); } - new SensorPrivacyService(mContext); + SensorPrivacyService service = new SensorPrivacyService(mContext); + if (startingUserId != -1) { + SystemService.TargetUser mockedTargetUser = + ExtendedMockito.mock(SystemService.TargetUser.class); + doReturn(startingUserId).when(mockedTargetUser).getUserIdentifier(); + service.onUserStarting(mockedTargetUser); + } onDeviceFile.delete(); } } From ca8057b7bc3d4af841ca708c1353728809714b9d Mon Sep 17 00:00:00 2001 From: Rodrigo Lagos Date: Tue, 28 Sep 2021 23:12:09 -0700 Subject: [PATCH 3/6] Add pre-grant bluetooth permissions for AUTOMOTIVE SetupWizard Bug: 201417592 Test: Tested on local builds Change-Id: I156c990d95b92295da5ede28aaea67f1aa63b596 (cherry picked from commit d0636e18d82d370942b57c4e8b86b29c5c45b6ec) Merged-in: I156c990d95b92295da5ede28aaea67f1aa63b596 --- .../pm/permission/DefaultPermissionGrantPolicy.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java b/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java index dab980a9e4b28..301914615562e 100644 --- a/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java +++ b/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java @@ -557,11 +557,14 @@ final class DefaultPermissionGrantPolicy { grantPermissionsToSystemPackage(pm, verifier, userId, PHONE_PERMISSIONS, SMS_PERMISSIONS); // SetupWizard - grantPermissionsToSystemPackage(pm, - ArrayUtils.firstOrNull(getKnownPackages( - PackageManagerInternal.PACKAGE_SETUP_WIZARD, userId)), userId, - PHONE_PERMISSIONS, CONTACTS_PERMISSIONS, ALWAYS_LOCATION_PERMISSIONS, - CAMERA_PERMISSIONS); + final String setupWizardPackage = ArrayUtils.firstOrNull(getKnownPackages( + PackageManagerInternal.PACKAGE_SETUP_WIZARD, userId)); + grantPermissionsToSystemPackage(pm, setupWizardPackage, userId, PHONE_PERMISSIONS, + CONTACTS_PERMISSIONS, ALWAYS_LOCATION_PERMISSIONS, CAMERA_PERMISSIONS); + if (mContext.getPackageManager().hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE, 0)) { + grantPermissionsToSystemPackage( + pm, setupWizardPackage, userId, NEARBY_DEVICES_PERMISSIONS); + } // Camera grantPermissionsToSystemPackage(pm, From f100f56cf00626f4a1148b73342677afc9ad7d63 Mon Sep 17 00:00:00 2001 From: Bill Yi Date: Thu, 7 Oct 2021 01:47:03 +0000 Subject: [PATCH 4/6] Import translations. DO NOT MERGE ANYWHERE Auto-generated-cl: translation import Change-Id: I153a51ca14636fc51e98234fd7c5a7b465336ced --- packages/SettingsLib/res/values-or/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/SettingsLib/res/values-or/strings.xml b/packages/SettingsLib/res/values-or/strings.xml index c885c16cb5bf4..8938fcf60473e 100644 --- a/packages/SettingsLib/res/values-or/strings.xml +++ b/packages/SettingsLib/res/values-or/strings.xml @@ -155,7 +155,7 @@ "ଉପଯୋଗକର୍ତ୍ତା: %1$s" "କିଛି ପୂର୍ବ-ନିର୍ଦ୍ଧାରିତ ମାନ ସେଟ୍‌ ହୋଇଛି" "କୌଣସି ଡିଫଲ୍ଟ ସେଟ୍‍ ହୋଇନାହିଁ" - "ଟେକ୍ସଟ-ରୁ-ସ୍ପିଚ୍ ସେଟିଂସ୍" + "ଟେକ୍ସଟ୍-ଟୁ-ସ୍ପିଚ୍ ସେଟିଂସ" "ଟେକ୍ସଟ୍‍-ଟୁ-ସ୍ପିଚ୍‍ ଆଉଟ୍‍ପୁଟ୍‌" "ସ୍ପିଚ୍‌ ରେଟ୍" "ଲେଖା ପଢ଼ିବାର ବେଗ" From 67927b8af5c0538d0e09cd691a11e26d96b8bfc9 Mon Sep 17 00:00:00 2001 From: Fiona Campbell Date: Fri, 24 Sep 2021 16:04:37 +0000 Subject: [PATCH 5/6] Stop RBC affecting brightness so drastically This change resets the brightness short term model when RBC is turned off or the strength is changed. We add a singular interaction when RBC is turned on, and add interactions in RBC as normal. Since users turning RBC in a bright environment could want any brightness between the given one and MAX, but turning RBC off will often result in a MAX interaction being logged, if the slider is already at MAX (given RBC) which is not necessarily what we want. In order to prevent unwanted changes to the brightness curve, we ignore interactions that turn RBC off. Turning RBC on seems like a valid indicator that the user wants the brightness darker, and at a specific brightness, however turning RBC off is less of an indicator. Changing the strength of RBC is also a good time to reset the short term model, however after resetting, we still add an interaction after this. Bug: 199187829 Test: manual Change-Id: I6512d9642f2da634aa60af6b5573c792c0a116d5 --- .../display/DisplayPowerController.java | 64 +++++++++++++------ 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java index abbe13ac260f8..e1a38572c32e7 100644 --- a/services/core/java/com/android/server/display/DisplayPowerController.java +++ b/services/core/java/com/android/server/display/DisplayPowerController.java @@ -125,6 +125,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call private static final int MSG_IGNORE_PROXIMITY = 8; private static final int MSG_STOP = 9; private static final int MSG_UPDATE_BRIGHTNESS = 10; + private static final int MSG_UPDATE_RBC = 11; private static final int PROXIMITY_UNKNOWN = -1; private static final int PROXIMITY_NEGATIVE = 0; @@ -422,13 +423,13 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call // PowerManager.BRIGHTNESS_INVALID_FLOAT when there's no temporary adjustment set. private float mTemporaryAutoBrightnessAdjustment; - // Whether a reduce bright colors (rbc) change has been initiated by the user. We want to - // retain the current backlight level when rbc is toggled, since rbc additionally makes the - // screen appear dimmer using screen colors rather than backlight levels, and therefore we - // don't actually want to compensate for this by then in/decreasing the backlight when - // toggling this feature. + // Whether reduce bright colors (rbc) has been turned on, or a change in strength has been + // requested. We want to retain the current backlight level when rbc is toggled, since rbc + // additionally makes the screen appear dimmer using screen colors rather than backlight levels, + // and therefore we don't actually want to compensate for this by then in/decreasing the + // backlight when toggling this feature. // This should be false during system start up. - private boolean mPendingUserRbcChange; + private boolean mPendingRbcOnOrChanged = false; // Animators. private ObjectAnimator mColorFadeOnAnimator; @@ -564,23 +565,35 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call @Override public void onReduceBrightColorsActivationChanged(boolean activated, boolean userInitiated) { - applyReduceBrightColorsSplineAdjustment(userInitiated); + applyReduceBrightColorsSplineAdjustment( + /* rbcStrengthChanged= */ false, activated); + } @Override public void onReduceBrightColorsStrengthChanged(int strength) { - applyReduceBrightColorsSplineAdjustment(/*userInitiated*/ false); + applyReduceBrightColorsSplineAdjustment( + /* rbcStrengthChanged= */ true, /* justActivated= */ false); } }); if (active) { - applyReduceBrightColorsSplineAdjustment(/*userInitiated*/ false); + applyReduceBrightColorsSplineAdjustment( + /* rbcStrengthChanged= */ false, /* justActivated= */ false); } } else { mCdsi = null; } } - private void applyReduceBrightColorsSplineAdjustment(boolean userInitiated) { + private void applyReduceBrightColorsSplineAdjustment( + boolean rbcStrengthChanged, boolean justActivated) { + final int strengthChanged = rbcStrengthChanged ? 1 : 0; + final int activated = justActivated ? 1 : 0; + mHandler.obtainMessage(MSG_UPDATE_RBC, strengthChanged, activated).sendToTarget(); + sendUpdatePowerState(); + } + + private void handleRbcChanged(boolean strengthChanged, boolean justActivated) { if (mBrightnessMapper == null) { Log.w(TAG, "No brightness mapping available to recalculate splines"); return; @@ -591,8 +604,13 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call adjustedNits[i] = mCdsi.getReduceBrightColorsAdjustedBrightnessNits(mNitsRange[i]); } mBrightnessMapper.recalculateSplines(mCdsi.isReduceBrightColorsActivated(), adjustedNits); - mPendingUserRbcChange = userInitiated; - sendUpdatePowerState(); + + mPendingRbcOnOrChanged = strengthChanged || justActivated; + + // Reset model if strength changed OR rbc is turned off + if (strengthChanged || !justActivated && mAutomaticBrightnessController != null) { + mAutomaticBrightnessController.resetShortTermModel(); + } } /** @@ -926,7 +944,8 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call private void reloadReduceBrightColours() { if (mCdsi != null && mCdsi.isReduceBrightColorsActivated()) { - applyReduceBrightColorsSplineAdjustment(/*userInitiated*/ false); + applyReduceBrightColorsSplineAdjustment( + /* rbcStrengthChanged= */ false, /* justActivated= */ false); } } @@ -2062,21 +2081,24 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call return true; } + // We want to return true if the user has set the screen brightness. + // If they have just turned RBC on (and therefore added that interaction to the curve), + // or changed the brightness another way, then we should return true. private boolean updateUserSetScreenBrightness() { - final boolean brightnessSplineChanged = mPendingUserRbcChange; - if (mPendingUserRbcChange && !Float.isNaN(mCurrentScreenBrightnessSetting)) { + final boolean treatAsIfUserChanged = mPendingRbcOnOrChanged; + if (treatAsIfUserChanged && !Float.isNaN(mCurrentScreenBrightnessSetting)) { mLastUserSetScreenBrightness = mCurrentScreenBrightnessSetting; } - mPendingUserRbcChange = false; + mPendingRbcOnOrChanged = false; if ((Float.isNaN(mPendingScreenBrightnessSetting) || mPendingScreenBrightnessSetting < 0.0f)) { - return brightnessSplineChanged; + return treatAsIfUserChanged; } if (mCurrentScreenBrightnessSetting == mPendingScreenBrightnessSetting) { mPendingScreenBrightnessSetting = PowerManager.BRIGHTNESS_INVALID_FLOAT; mTemporaryScreenBrightness = PowerManager.BRIGHTNESS_INVALID_FLOAT; - return brightnessSplineChanged; + return treatAsIfUserChanged; } setCurrentScreenBrightness(mPendingScreenBrightnessSetting); mLastUserSetScreenBrightness = mPendingScreenBrightnessSetting; @@ -2406,6 +2428,12 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call } handleSettingsChange(false /*userSwitch*/); break; + + case MSG_UPDATE_RBC: + final int strengthChanged = msg.arg1; + final int justActivated = msg.arg2; + handleRbcChanged(strengthChanged == 1, justActivated == 1); + break; } } } From a44514e9616cd7bcef7c31afaaebc1b3f4c1af2e Mon Sep 17 00:00:00 2001 From: Anthony Stange Date: Wed, 6 Oct 2021 19:21:43 +0000 Subject: [PATCH 6/6] Prevent multiple outstanding permission queries per client Under stress tests, it's possible for several messages to be sent before CHRE has time to respond to the permission query which can cause several permission queries to be sent and CHRE to be overwhelmed. Fixes: 202201157 Test: Flash device and run stress test Change-Id: I16871a116216cd288f0eba0e6c5572f5afa6b959 --- .../contexthub/ContextHubClientBroker.java | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/location/contexthub/ContextHubClientBroker.java b/services/core/java/com/android/server/location/contexthub/ContextHubClientBroker.java index fa33338a61e72..03e421bfed677 100644 --- a/services/core/java/com/android/server/location/contexthub/ContextHubClientBroker.java +++ b/services/core/java/com/android/server/location/contexthub/ContextHubClientBroker.java @@ -209,6 +209,12 @@ public class ContextHubClientBroker extends IContextHubClient.Stub */ private AtomicBoolean mIsPendingIntentCancelled = new AtomicBoolean(false); + /** + * True if a permissions query has been issued and is being processed. Used to prevent too many + * queries from being issued by a single client at once. + */ + private AtomicBoolean mIsPermQueryIssued = new AtomicBoolean(false); + /* * True if the application creating the client has the ACCESS_CONTEXT_HUB permission. */ @@ -240,11 +246,11 @@ public class ContextHubClientBroker extends IContextHubClient.Stub private final IContextHubTransactionCallback mQueryPermsCallback = new IContextHubTransactionCallback.Stub() { @Override - public void onTransactionComplete(int result) { - } + public void onTransactionComplete(int result) {} @Override public void onQueryResponse(int result, List nanoAppStateList) { + mIsPermQueryIssued.set(false); if (result != ContextHubTransaction.RESULT_SUCCESS && nanoAppStateList != null) { Log.e(TAG, "Permissions query failed, but still received nanoapp state"); } else if (nanoAppStateList != null) { @@ -656,9 +662,11 @@ public class ContextHubClientBroker extends IContextHubClient.Stub * communicated with in the past. */ private void checkNanoappPermsAsync() { - ContextHubServiceTransaction transaction = mTransactionManager.createQueryTransaction( - mAttachedContextHubInfo.getId(), mQueryPermsCallback, mPackage); - mTransactionManager.addTransaction(transaction); + if (!mIsPermQueryIssued.getAndSet(true)) { + ContextHubServiceTransaction transaction = mTransactionManager.createQueryTransaction( + mAttachedContextHubInfo.getId(), mQueryPermsCallback, mPackage); + mTransactionManager.addTransaction(transaction); + } } private int updateNanoAppAuthState(