Merge "Ensure pkg uid matches provided uid for device phone number check" into sc-qpr1-dev am: e2ba264dd0

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/15926751

Change-Id: Id34577e8237072757ebd832283cb9a39ee725011
This commit is contained in:
TreeHugger Robot
2021-10-07 00:10:03 +00:00
committed by Automerger Merge Worker
2 changed files with 118 additions and 10 deletions

View File

@@ -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);
}
}
}

View File

@@ -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.
*
* <p>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.
*
* <p>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);