diff options
2 files changed, 62 insertions, 77 deletions
diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index fbaf1ce25b85..7b941d1aa19b 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -5760,36 +5760,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return new ParcelableGranteeMap(result); } - /** - * Enforce one the following conditions are met: - * (1) The device has a Device Owner, and one of the following holds: - * (1.1) The caller is the Device Owner - * (1.2) The caller is another app in the same user as the device owner, AND - * The caller is the delegated certificate installer. - * (1.3) The caller is a Profile Owner and the calling user is affiliated. - * (2) The user has a profile owner, AND: - * (2.1) The profile owner has been granted access to Device IDs and one of the following - * holds: - * (2.1.1) The caller is the profile owner. - * (2.1.2) The caller is from another app in the same user as the profile owner, AND - * (2.1.2.1) The caller is the delegated cert installer. - * - * For the device owner case, simply check that the caller is the device owner or the - * delegated certificate installer. - * - * For the profile owner case, first check that the caller is the profile owner or can - * manage the DELEGATION_CERT_INSTALL scope. - * If that check succeeds, ensure the profile owner was granted access to device - * identifiers. The grant is transitive: The delegated cert installer is implicitly allowed - * access to device identifiers in this case as part of the delegation. - */ - @VisibleForTesting - public void enforceCallerCanRequestDeviceIdAttestation(CallerIdentity caller) - throws SecurityException { - Preconditions.checkCallAuthorization(hasDeviceIdAccessUnchecked(caller.getPackageName(), - caller.getUid())); - } - @VisibleForTesting public static int[] translateIdAttestationFlags( int idAttestationFlags) { @@ -5844,8 +5814,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { final boolean isCallerDelegate = isCallerDelegate(caller, DELEGATION_CERT_INSTALL); final boolean isCredentialManagementApp = isCredentialManagementApp(caller); if (deviceIdAttestationRequired && attestationUtilsFlags.length > 0) { - // TODO: replace enforce methods - enforceCallerCanRequestDeviceIdAttestation(caller); + Preconditions.checkCallAuthorization(hasDeviceIdAccessUnchecked( + caller.getPackageName(), caller.getUid())); enforceIndividualAttestationSupportedIfRequested(attestationUtilsFlags); } else { Preconditions.checkCallAuthorization((caller.hasAdminComponent() @@ -9376,21 +9346,36 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } /** - * Check if caller is device owner, delegate cert installer or profile owner of - * affiliated user. Or if caller is profile owner for a specified user or delegate cert - * installer on an organization-owned device. + * Check if one the following conditions hold: + * (1) The device has a Device Owner, and one of the following holds: + * (1.1) The caller is the Device Owner + * (1.2) The caller is another app in the same user as the device owner, AND + * The caller is the delegated certificate installer. + * (1.3) The caller is a Profile Owner and the calling user is affiliated. + * (2) The user has a profile owner, AND: + * (2.1) The profile owner has been granted access to Device IDs and one of the following + * holds: + * (2.1.1) The caller is the profile owner. + * (2.1.2) The caller is from another app in the same user as the profile owner, AND + * the caller is the delegated cert installer. + * + * For the device owner case, simply check that the caller is the device owner or the + * delegated certificate installer. + * + * For the profile owner case, first check that the caller is the profile owner or can + * manage the DELEGATION_CERT_INSTALL scope. + * If that check succeeds, ensure the profile owner was granted access to device + * identifiers. The grant is transitive: The delegated cert installer is implicitly allowed + * access to device identifiers in this case as part of the delegation. */ - private boolean hasDeviceIdAccessUnchecked(String packageName, int uid) { - // Is the caller a device owner, delegate cert installer or profile owner of an - // affiliated user. + @VisibleForTesting + boolean hasDeviceIdAccessUnchecked(String packageName, int uid) { ComponentName deviceOwner = getDeviceOwnerComponent(true); if (deviceOwner != null && (deviceOwner.getPackageName().equals(packageName) || isCallerDelegate(packageName, uid, DELEGATION_CERT_INSTALL))) { return true; } final int userId = UserHandle.getUserId(uid); - // Is the caller the profile owner for the specified user, or delegate cert installer on an - // organization-owned device. ComponentName profileOwner = getProfileOwnerAsUser(userId); final boolean isCallerProfileOwnerOrDelegate = profileOwner != null && (profileOwner.getPackageName().equals(packageName) diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index a7ca08385b9f..eda91330547e 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -6724,55 +6724,51 @@ public class DevicePolicyManagerTest extends DpmTestBase { } @Test - public void testEnforceCallerCanRequestDeviceIdAttestation_deviceOwnerCaller() + public void testHasDeviceIdAccessUnchecked_deviceOwnerCaller() throws Exception { mContext.binder.callingUid = DpmMockContext.CALLER_SYSTEM_USER_UID; setupDeviceOwner(); configureContextForAccess(mContext, false); // Device owner should be allowed to request Device ID attestation. - dpms.enforceCallerCanRequestDeviceIdAttestation(dpms.getCallerIdentity(admin1)); + assertThat(dpms.hasDeviceIdAccessUnchecked( + admin1.getPackageName(), + DpmMockContext.CALLER_SYSTEM_USER_UID)).isTrue(); - mContext.binder.callingUid = DpmMockContext.ANOTHER_UID; // Another package must not be allowed to request Device ID attestation. - assertExpectException(SecurityException.class, null, - () -> dpms.enforceCallerCanRequestDeviceIdAttestation( - dpms.getCallerIdentity(null, admin2.getPackageName()))); - - // Another component that is not the admin must not be allowed to request Device ID - // attestation. - assertExpectException(SecurityException.class, null, - () -> dpms.enforceCallerCanRequestDeviceIdAttestation( - dpms.getCallerIdentity(admin2))); + assertThat(dpms.hasDeviceIdAccessUnchecked( + DpmMockContext.ANOTHER_PACKAGE_NAME, + DpmMockContext.CALLER_SYSTEM_USER_UID)).isFalse(); } @Test - public void testEnforceCallerCanRequestDeviceIdAttestation_profileOwnerCaller() + public void testHasDeviceIdAccessUnchecked_profileOwnerCaller() throws Exception { configureContextForAccess(mContext, false); // Make sure a security exception is thrown if the device has no profile owner. - assertExpectException(SecurityException.class, null, - () -> dpms.enforceCallerCanRequestDeviceIdAttestation( - dpms.getCallerIdentity(admin1))); + assertThat(dpms.hasDeviceIdAccessUnchecked( + admin1.getPackageName(), + DpmMockContext.CALLER_UID)).isFalse(); setupProfileOwner(); configureProfileOwnerOfOrgOwnedDevice(admin1, CALLER_USER_HANDLE); // The profile owner is allowed to request Device ID attestation. - mServiceContext.binder.callingUid = DpmMockContext.CALLER_UID; - dpms.enforceCallerCanRequestDeviceIdAttestation(dpms.getCallerIdentity(admin1)); + assertThat(dpms.hasDeviceIdAccessUnchecked( + admin1.getPackageName(), + DpmMockContext.CALLER_UID)).isTrue(); // But not another package. - mContext.binder.callingUid = DpmMockContext.ANOTHER_UID; - assertExpectException(SecurityException.class, null, - () -> dpms.enforceCallerCanRequestDeviceIdAttestation( - dpms.getCallerIdentity(null, admin2.getPackageName()))); + assertThat(dpms.hasDeviceIdAccessUnchecked( + DpmMockContext.ANOTHER_PACKAGE_NAME, + DpmMockContext.CALLER_UID)).isFalse(); // Or another component which is not the admin. - assertExpectException(SecurityException.class, null, - () -> dpms.enforceCallerCanRequestDeviceIdAttestation( - dpms.getCallerIdentity(admin2, admin2.getPackageName()))); + mContext.binder.callingUid = DpmMockContext.ANOTHER_UID; + assertThat(dpms.hasDeviceIdAccessUnchecked( + admin1.getPackageName(), + DpmMockContext.ANOTHER_UID)).isFalse(); } public void runAsDelegatedCertInstaller(DpmRunnable action) throws Exception { @@ -6788,7 +6784,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { } @Test - public void testEnforceCallerCanRequestDeviceIdAttestation_delegateCaller() throws Exception { + public void testHasDeviceIdAccessUnchecked_delegateCaller() throws Exception { setupProfileOwner(); markDelegatedCertInstallerAsInstalled(); @@ -6800,15 +6796,19 @@ public class DevicePolicyManagerTest extends DpmTestBase { configureProfileOwnerOfOrgOwnedDevice(admin1, CALLER_USER_HANDLE); // Make sure that the profile owner can still request Device ID attestation. - mServiceContext.binder.callingUid = DpmMockContext.CALLER_UID; - dpms.enforceCallerCanRequestDeviceIdAttestation(dpms.getCallerIdentity(admin1)); + assertThat(dpms.hasDeviceIdAccessUnchecked( + admin1.getPackageName(), + DpmMockContext.CALLER_UID)).isTrue(); - runAsDelegatedCertInstaller(dpm -> dpms.enforceCallerCanRequestDeviceIdAttestation( - dpms.getCallerIdentity(null, DpmMockContext.DELEGATE_PACKAGE_NAME))); + runAsDelegatedCertInstaller(dpm -> assertThat( + dpms.hasDeviceIdAccessUnchecked( + DpmMockContext.DELEGATE_PACKAGE_NAME, + UserHandle.getUid(CALLER_USER_HANDLE, + DpmMockContext.DELEGATE_CERT_INSTALLER_UID))).isTrue()); } @Test - public void testEnforceCallerCanRequestDeviceIdAttestation_delegateCallerWithoutPermissions() + public void testHasDeviceIdAccessUnchecked_delegateCallerWithoutPermissions() throws Exception { setupProfileOwner(); markDelegatedCertInstallerAsInstalled(); @@ -6818,14 +6818,14 @@ public class DevicePolicyManagerTest extends DpmTestBase { dpm -> dpm.setDelegatedScopes(admin1, DpmMockContext.DELEGATE_PACKAGE_NAME, Arrays.asList(DELEGATION_CERT_INSTALL))); - assertExpectException(SecurityException.class, null, - () -> dpms.enforceCallerCanRequestDeviceIdAttestation( - dpms.getCallerIdentity(admin1))); + assertThat(dpms.hasDeviceIdAccessUnchecked( + admin1.getPackageName(), + DpmMockContext.CALLER_UID)).isFalse(); runAsDelegatedCertInstaller(dpm -> { - assertExpectException(SecurityException.class, /* messageRegex= */ null, - () -> dpms.enforceCallerCanRequestDeviceIdAttestation( - dpms.getCallerIdentity(null, DpmMockContext.DELEGATE_PACKAGE_NAME))); + assertThat(dpms.hasDeviceIdAccessUnchecked( + DpmMockContext.DELEGATE_PACKAGE_NAME, + DpmMockContext.CALLER_UID)).isFalse(); }); } |