diff options
7 files changed, 405 insertions, 40 deletions
diff --git a/api/system-current.txt b/api/system-current.txt index 8586d5a7e14f..0ca52310a1ed 100755 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -861,7 +861,6 @@ package android.app.admin { method @Nullable public android.content.ComponentName getProfileOwner() throws java.lang.IllegalArgumentException; method @Nullable @RequiresPermission(android.Manifest.permission.MANAGE_USERS) public String getProfileOwnerNameAsUser(int) throws java.lang.IllegalArgumentException; method @RequiresPermission(android.Manifest.permission.MANAGE_USERS) public int getUserProvisioningState(); - method public boolean hasDeviceIdentifierAccess(@NonNull String, int, int); method public boolean isDeviceManaged(); method @RequiresPermission(android.Manifest.permission.MANAGE_USERS) public boolean isDeviceProvisioned(); method @RequiresPermission(android.Manifest.permission.MANAGE_USERS) public boolean isDeviceProvisioningConfigApplied(); @@ -8815,6 +8814,7 @@ package android.permission { } public final class PermissionManager { + method public int checkDeviceIdentifierAccess(@Nullable String, @Nullable String, @Nullable String, int, int); method @NonNull @RequiresPermission(android.Manifest.permission.ADJUST_RUNTIME_PERMISSIONS_POLICY) public java.util.Set<java.lang.String> getAutoRevokeExemptionGrantedPackages(); method @NonNull @RequiresPermission(android.Manifest.permission.ADJUST_RUNTIME_PERMISSIONS_POLICY) public java.util.Set<java.lang.String> getAutoRevokeExemptionRequestedPackages(); method @IntRange(from=0) @RequiresPermission(anyOf={android.Manifest.permission.ADJUST_RUNTIME_PERMISSIONS_POLICY, android.Manifest.permission.UPGRADE_RUNTIME_PERMISSIONS}) public int getRuntimePermissionsVersion(); diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index 99b730e89fd4..51cfa31ae4cf 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -7071,7 +7071,6 @@ public class DevicePolicyManager { * * @hide */ - @SystemApi public boolean hasDeviceIdentifierAccess(@NonNull String packageName, int pid, int uid) { throwIfParentInstance("hasDeviceIdentifierAccess"); if (packageName == null) { diff --git a/core/java/android/permission/IPermissionManager.aidl b/core/java/android/permission/IPermissionManager.aidl index 09df72c286d0..235b0830b9aa 100644 --- a/core/java/android/permission/IPermissionManager.aidl +++ b/core/java/android/permission/IPermissionManager.aidl @@ -54,6 +54,8 @@ interface IPermissionManager { int checkUidPermission(String permName, int uid); + int checkDeviceIdentifierAccess(String packageName, String callingFeatureId, String message, int pid, int uid); + void addOnPermissionsChangeListener(in IOnPermissionsChangeListener listener); void removeOnPermissionsChangeListener(in IOnPermissionsChangeListener listener); diff --git a/core/java/android/permission/PermissionManager.java b/core/java/android/permission/PermissionManager.java index fc993b87a978..3dc8e9924444 100644 --- a/core/java/android/permission/PermissionManager.java +++ b/core/java/android/permission/PermissionManager.java @@ -40,6 +40,7 @@ import android.os.UserHandle; import android.util.Slog; import com.android.internal.annotations.Immutable; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.CollectionUtils; import java.util.ArrayList; @@ -84,10 +85,25 @@ public final class PermissionManager { */ public PermissionManager(@NonNull Context context, IPackageManager packageManager) throws ServiceManager.ServiceNotFoundException { + this(context, packageManager, IPermissionManager.Stub.asInterface( + ServiceManager.getServiceOrThrow("permissionmgr"))); + } + + /** + * Creates a new instance with the provided instantiation of the IPermissionManager. + * + * @param context the current context in which to operate + * @param packageManager package manager service to be used for package related permission + * requests + * @param permissionManager injectable permission manager service + * @hide + */ + @VisibleForTesting + public PermissionManager(@NonNull Context context, IPackageManager packageManager, + IPermissionManager permissionManager) { mContext = context; mPackageManager = packageManager; - mPermissionManager = IPermissionManager.Stub.asInterface( - ServiceManager.getServiceOrThrow("permissionmgr")); + mPermissionManager = permissionManager; } /** @@ -486,6 +502,30 @@ public final class PermissionManager { } } + /** + * Checks whether the package with the given pid/uid can read device identifiers. + * + * @param packageName the name of the package to be checked for identifier access + * @param message the message to be used for logging during identifier access + * verification + * @param callingFeatureId the feature in the package + * @param pid the process id of the package to be checked + * @param uid the uid of the package to be checked + * @return {@link PackageManager#PERMISSION_GRANTED} if the package is allowed identifier + * access, {@link PackageManager#PERMISSION_DENIED} otherwise + * @hide + */ + @SystemApi + public int checkDeviceIdentifierAccess(@Nullable String packageName, @Nullable String message, + @Nullable String callingFeatureId, int pid, int uid) { + try { + return mPermissionManager.checkDeviceIdentifierAccess(packageName, message, + callingFeatureId, pid, uid); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + /* @hide */ private static int checkPermissionUncached(@Nullable String permission, int pid, int uid) { final IActivityManager am = ActivityManager.getService(); diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index 5d6eaf22dae2..79d0c2db4448 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -59,6 +59,7 @@ import android.app.AppOpsManager; import android.app.ApplicationPackageManager; import android.app.IActivityManager; import android.app.admin.DeviceAdminInfo; +import android.app.admin.DevicePolicyManager; import android.app.admin.DevicePolicyManagerInternal; import android.compat.annotation.ChangeId; import android.compat.annotation.EnabledAfter; @@ -113,6 +114,7 @@ import android.util.SparseArray; import android.util.SparseBooleanArray; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.compat.IPlatformCompat; import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; @@ -243,6 +245,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { @GuardedBy("mLock") private final PermissionSettings mSettings; + /** Injector that can be used to facilitate testing. */ + private final Injector mInjector; + @GuardedBy("mLock") private ArraySet<String> mPrivappPermissionsViolations; @@ -352,10 +357,17 @@ public class PermissionManagerService extends IPermissionManager.Stub { PermissionManagerService(Context context, @NonNull Object externalLock) { + this(context, externalLock, new Injector(context)); + } + + @VisibleForTesting + PermissionManagerService(Context context, @NonNull Object externalLock, + @NonNull Injector injector) { + mInjector = injector; // The package info cache is the cache for package and permission information. - PackageManager.invalidatePackageInfoCache(); - PermissionManager.disablePermissionCache(); - PermissionManager.disablePackageNamePermissionCache(); + mInjector.invalidatePackageInfoCache(); + mInjector.disablePermissionCache(); + mInjector.disablePackageNamePermissionCache(); mContext = context; mLock = externalLock; @@ -952,6 +964,59 @@ public class PermissionManagerService extends IPermissionManager.Stub { } @Override + public int checkDeviceIdentifierAccess(@Nullable String packageName, @Nullable String message, + @Nullable String callingFeatureId, int pid, int uid) { + // If the check is being requested by an app then only allow the app to query its own + // access status. + int callingUid = mInjector.getCallingUid(); + int callingPid = mInjector.getCallingPid(); + if (UserHandle.getAppId(callingUid) >= Process.FIRST_APPLICATION_UID && (callingUid != uid + || callingPid != pid)) { + String response = String.format( + "Calling uid %d, pid %d cannot check device identifier access for package %s " + + "(uid=%d, pid=%d)", + callingUid, callingPid, packageName, uid, pid); + Log.w(TAG, response); + throw new SecurityException(response); + } + // Allow system and root access to the device identifiers. + final int appId = UserHandle.getAppId(uid); + if (appId == Process.SYSTEM_UID || appId == Process.ROOT_UID) { + return PackageManager.PERMISSION_GRANTED; + } + // Allow access to packages that have the READ_PRIVILEGED_PHONE_STATE permission. + if (mInjector.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, pid, + uid) == PackageManager.PERMISSION_GRANTED) { + return PackageManager.PERMISSION_GRANTED; + } + // If the calling package is not null then perform the appop and device / profile owner + // check. + if (packageName != null) { + // Allow access to a package that has been granted the READ_DEVICE_IDENTIFIERS appop. + long token = mInjector.clearCallingIdentity(); + AppOpsManager appOpsManager = (AppOpsManager) mInjector.getSystemService( + Context.APP_OPS_SERVICE); + try { + if (appOpsManager.noteOpNoThrow(AppOpsManager.OPSTR_READ_DEVICE_IDENTIFIERS, uid, + packageName, callingFeatureId, message) == AppOpsManager.MODE_ALLOWED) { + return PackageManager.PERMISSION_GRANTED; + } + } finally { + mInjector.restoreCallingIdentity(token); + } + // Check if the calling packages meets the device / profile owner requirements for + // identifier access. + DevicePolicyManager devicePolicyManager = + (DevicePolicyManager) mInjector.getSystemService(Context.DEVICE_POLICY_SERVICE); + if (devicePolicyManager != null && devicePolicyManager.hasDeviceIdentifierAccess( + packageName, pid, uid)) { + return PackageManager.PERMISSION_GRANTED; + } + } + return PackageManager.PERMISSION_DENIED; + } + + @Override public void addOnPermissionsChangeListener(IOnPermissionsChangeListener listener) { mContext.enforceCallingOrSelfPermission( Manifest.permission.OBSERVE_GRANT_REVOKE_PERMISSIONS, @@ -4797,4 +4862,94 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } } + + /** + * Allows injection of services and method responses to facilitate testing. + * + * <p>Test classes can create a mock of this class and pass it to the PermissionManagerService + * constructor to control behavior of services and external methods during execution. + * @hide + */ + @VisibleForTesting + public static class Injector { + private final Context mContext; + + /** + * Public constructor that accepts a {@code context} within which to operate. + */ + public Injector(@NonNull Context context) { + mContext = context; + } + + /** + * Returns the UID of the calling package. + */ + public int getCallingUid() { + return Binder.getCallingUid(); + } + + /** + * Returns the process ID of the calling package. + */ + public int getCallingPid() { + return Binder.getCallingPid(); + } + + /** + * Invalidates the package info cache. + */ + public void invalidatePackageInfoCache() { + PackageManager.invalidatePackageInfoCache(); + } + + /** + * Disables the permission cache. + */ + public void disablePermissionCache() { + PermissionManager.disablePermissionCache(); + } + + /** + * Disables the package name permission cache. + */ + public void disablePackageNamePermissionCache() { + PermissionManager.disablePackageNamePermissionCache(); + } + + /** + * Checks if the package running under the specified {@code pid} and {@code uid} has been + * granted the provided {@code permission}. + * + * @return {@link PackageManager#PERMISSION_GRANTED} if the package has been granted the + * permission, {@link PackageManager#PERMISSION_DENIED} otherwise + */ + public int checkPermission(@NonNull String permission, int pid, int uid) { + return mContext.checkPermission(permission, pid, uid); + } + + /** + * Clears the calling identity to allow subsequent calls to be treated as coming from this + * package. + * + * @return a token that can be used to restore the calling identity + */ + public long clearCallingIdentity() { + return Binder.clearCallingIdentity(); + } + + /** + * Restores the calling identity to that of the calling package based on the provided + * {@code token}. + */ + public void restoreCallingIdentity(long token) { + Binder.restoreCallingIdentity(token); + } + + /** + * Returns the system service with the provided {@code name}. + */ + public Object getSystemService(@NonNull String name) { + return mContext.getSystemService(name); + } + } } diff --git a/services/tests/servicestests/src/com/android/server/pm/permission/PermissionManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/pm/permission/PermissionManagerServiceTest.java new file mode 100644 index 000000000000..56966776c8fb --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/pm/permission/PermissionManagerServiceTest.java @@ -0,0 +1,197 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.pm.permission; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertThrows; + +import android.app.AppOpsManager; +import android.app.admin.DevicePolicyManager; +import android.content.Context; +import android.content.pm.PackageManager; +import android.os.Process; +import android.permission.PermissionManagerInternal; + +import androidx.test.InstrumentationRegistry; +import androidx.test.runner.AndroidJUnit4; + +import com.android.server.LocalServices; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +@RunWith(AndroidJUnit4.class) +public class PermissionManagerServiceTest { + private static final String TAG = "PermissionManagerServiceTag"; + + private static final int SYSTEM_UID = 1000; + private static final int SYSTEM_PID = 1234; + private static final int APP_UID = Process.FIRST_APPLICATION_UID; + private static final int APP_PID = 5678; + + private PermissionManagerService mPermissionManagerService; + private Context mContext; + + @Mock + private PermissionManagerService.Injector mInjector; + + @Mock + private AppOpsManager mAppOpsManager; + + @Mock + private DevicePolicyManager mDevicePolicyManager; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + + mContext = InstrumentationRegistry.getContext(); + Object lock = new Object(); + mPermissionManagerService = new PermissionManagerService(mContext, lock, mInjector); + } + + @After + public void tearDown() { + // The LocalServices added by the constructor of the PermissionManagerService can either be + // removed here after each test when tests are run serially, or to run them in parallel + // the Injector can provide methods to add these that can be ignored by the mock. + LocalServices.removeServiceForTest(PermissionManagerServiceInternal.class); + LocalServices.removeServiceForTest(PermissionManagerInternal.class); + } + + @Test + public void checkDeviceIdentifierAccess_callingAppUidMismatch_throwsException() { + // An application should only be able to query its own device identifier access, querying + // of any other UIDs should result in a SecurityException. + setupCheckDeviceIdentifierAccessTest(APP_PID, APP_UID); + + assertThrows(SecurityException.class, + () -> mPermissionManagerService.checkDeviceIdentifierAccess( + mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, + APP_PID, SYSTEM_UID)); + } + + @Test + public void checkDeviceIdentifierAccess_callingAppPidMismatch_throwsException() { + // Similar to above an app can only specify its own pid, a mismatch should result in a + // SecurityException. + setupCheckDeviceIdentifierAccessTest(APP_PID, APP_UID); + + assertThrows(SecurityException.class, + () -> mPermissionManagerService.checkDeviceIdentifierAccess( + mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, + SYSTEM_PID, APP_UID)); + } + + @Test + public void checkDeviceIdentifierAccess_callingAppIdWithoutAccess_returnsDenied() { + // An application can query its own device identifier access; this test verifies that all + // checks can run through completion and return denied. + setupCheckDeviceIdentifierAccessTest(APP_PID, APP_UID); + + int result = mPermissionManagerService.checkDeviceIdentifierAccess( + mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, APP_PID, + APP_UID); + + assertEquals(PackageManager.PERMISSION_DENIED, result); + } + + @Test + public void checkDeviceIdentifierAccess_systemUid_returnsGranted() { + // The system UID should always have access to device identifiers. + setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID); + int result = mPermissionManagerService.checkDeviceIdentifierAccess( + mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, SYSTEM_PID, + SYSTEM_UID); + + assertEquals(PackageManager.PERMISSION_GRANTED, result); + } + + @Test + public void checkDeviceIdentifierAccess_hasPrivilegedPermission_returnsGranted() { + // Apps with the READ_PRIVILEGED_PHONE_STATE permission should have access to device + // identifiers. + setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID); + when(mInjector.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, + APP_PID, APP_UID)).thenReturn(PackageManager.PERMISSION_GRANTED); + + int result = mPermissionManagerService.checkDeviceIdentifierAccess( + mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, APP_PID, + APP_UID); + + assertEquals(PackageManager.PERMISSION_GRANTED, result); + } + + @Test + 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); + when(mAppOpsManager.noteOpNoThrow(eq(AppOpsManager.OPSTR_READ_DEVICE_IDENTIFIERS), + eq(APP_UID), eq(mContext.getPackageName()), any(), any())).thenReturn( + AppOpsManager.MODE_ALLOWED); + + int result = mPermissionManagerService.checkDeviceIdentifierAccess( + mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, APP_PID, + APP_UID); + + assertEquals(PackageManager.PERMISSION_GRANTED, result); + } + + @Test + 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); + when(mDevicePolicyManager.hasDeviceIdentifierAccess(mContext.getPackageName(), APP_PID, + APP_UID)).thenReturn(true); + + int result = mPermissionManagerService.checkDeviceIdentifierAccess( + mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, APP_PID, + APP_UID); + + assertEquals(PackageManager.PERMISSION_GRANTED, result); + } + + private void setupCheckDeviceIdentifierAccessTest(int callingPid, int callingUid) { + when(mInjector.getCallingPid()).thenReturn(callingPid); + when(mInjector.getCallingUid()).thenReturn(callingUid); + + // Configure the checkDeviceIdentifierAccess tests to fail all access checks, then each test + // can individually set the access check to pass for verification. + when(mInjector.checkPermission(anyString(), anyInt(), anyInt())).thenReturn( + PackageManager.PERMISSION_DENIED); + + when(mAppOpsManager.noteOpNoThrow(anyString(), anyInt(), anyString(), any(), + any())).thenReturn(AppOpsManager.MODE_DEFAULT); + when(mInjector.getSystemService(eq(Context.APP_OPS_SERVICE))).thenReturn(mAppOpsManager); + + when(mDevicePolicyManager.hasDeviceIdentifierAccess(anyString(), anyInt(), + anyInt())).thenReturn(false); + when(mInjector.getSystemService(eq(Context.DEVICE_POLICY_SERVICE))).thenReturn( + mDevicePolicyManager); + } +} diff --git a/telephony/common/com/android/internal/telephony/TelephonyPermissions.java b/telephony/common/com/android/internal/telephony/TelephonyPermissions.java index 68b17688c22e..0b331744d922 100644 --- a/telephony/common/com/android/internal/telephony/TelephonyPermissions.java +++ b/telephony/common/com/android/internal/telephony/TelephonyPermissions.java @@ -20,7 +20,6 @@ import static android.content.pm.PackageManager.PERMISSION_GRANTED; import android.Manifest; import android.annotation.Nullable; import android.app.AppOpsManager; -import android.app.admin.DevicePolicyManager; import android.content.Context; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; @@ -28,6 +27,7 @@ import android.os.Binder; import android.os.Build; import android.os.Process; import android.os.UserHandle; +import android.permission.PermissionManager; import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; import android.util.Log; @@ -303,14 +303,10 @@ public final class TelephonyPermissions { String message, boolean allowCarrierPrivilegeOnAnySub) { int uid = Binder.getCallingUid(); int pid = Binder.getCallingPid(); - // Allow system and root access to the device identifiers. - final int appId = UserHandle.getAppId(uid); - if (appId == Process.SYSTEM_UID || appId == Process.ROOT_UID) { - return true; - } - // Allow access to packages that have the READ_PRIVILEGED_PHONE_STATE permission. - if (context.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, pid, - uid) == PackageManager.PERMISSION_GRANTED) { + PermissionManager permissionManager = (PermissionManager) context.getSystemService( + Context.PERMISSION_SERVICE); + if (permissionManager.checkDeviceIdentifierAccess(callingPackage, message, callingFeatureId, + pid, uid) == PackageManager.PERMISSION_GRANTED) { return true; } @@ -323,30 +319,6 @@ public final class TelephonyPermissions { return true; } - // if the calling package is not null then perform the DevicePolicyManager device / - // profile owner and Appop checks. - if (callingPackage != null) { - // Allow access to an app that has been granted the READ_DEVICE_IDENTIFIERS app op. - long token = Binder.clearCallingIdentity(); - AppOpsManager appOpsManager = (AppOpsManager) context.getSystemService( - Context.APP_OPS_SERVICE); - try { - if (appOpsManager.noteOpNoThrow(AppOpsManager.OPSTR_READ_DEVICE_IDENTIFIERS, uid, - callingPackage, callingFeatureId, null) == AppOpsManager.MODE_ALLOWED) { - return true; - } - } finally { - Binder.restoreCallingIdentity(token); - } - // Allow access to a device / profile owner app. - DevicePolicyManager devicePolicyManager = - (DevicePolicyManager) context.getSystemService( - Context.DEVICE_POLICY_SERVICE); - if (devicePolicyManager != null && devicePolicyManager.hasDeviceIdentifierAccess( - callingPackage, pid, uid)) { - return true; - } - } return reportAccessDeniedToReadIdentifiers(context, subId, pid, uid, callingPackage, message); } |