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: Iede9a11f1e2333eac3c9630e6682d8cbef26c4c0
This commit is contained in:
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user