From 713cf9ac1261b5615b17a7da02103e8d23db04d2 Mon Sep 17 00:00:00 2001
From: Eghosa Ewansiha-Vlachavas <eevlachavas@google.com>
Date: Mon, 22 Aug 2022 14:55:57 +0000
Subject: [PATCH] Merge check Device ID logic in DPMS

checkDeviceIdentifierAccess() and
enforceCallerCanRequestDeviceIdAttestation() overlap in terms of
checking if the caller has acces to the device ID. However they each use
different logic for this check. To ensure these two checks are always
in sync, they should call the same underlying code wherever logic can overlap.

Fixes: 220721469
Test: atest com.android.server.devicepolicy.DevicePolicyManagerTest
Test: atest com.android.cts.devicepolicy.MixedManagedProfileOwnerTest#testKeyManagement
Test: atest com.android.cts.devicepolicy.MixedDeviceOwnerTest#testDelegatedCertInstallerDirectly
Test:atest android.admin.cts.DevicePolicyManagerTest
Change-Id: Ibe196123640c1d2d29fd62c7c77453c0874fc7a3
---
 .../DevicePolicyManagerService.java           | 45 +++++++------------
 1 file changed, 15 insertions(+), 30 deletions(-)

diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
index c1c7bceaebd9c..8caad21d3efe9 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -5786,29 +5786,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
     @VisibleForTesting
     public void enforceCallerCanRequestDeviceIdAttestation(CallerIdentity caller)
             throws SecurityException {
-        /**
-         *  First check if there's a profile owner because the device could be in COMP mode (where
-         *  there's a device owner and profile owner on the same device).
-         *  If the caller is from the work profile, then it must be the PO or the delegate, and
-         *  it must have the right permission to access device identifiers.
-         */
-        int callerUserId = caller.getUserId();
-        if (hasProfileOwner(callerUserId)) {
-            // Make sure that the caller is the profile owner or delegate.
-            Preconditions.checkCallAuthorization(canInstallCertificates(caller));
-            // Verify that the managed profile is on an organization-owned device (or is affiliated
-            // with the device owner user) and as such the profile owner can access Device IDs.
-            if (isProfileOwnerOfOrganizationOwnedDevice(callerUserId)
-                    || isUserAffiliatedWithDevice(callerUserId)) {
-                return;
-            }
-            throw new SecurityException(
-                    "Profile Owner is not allowed to access Device IDs.");
-        }
-
-        // If not, fall back to the device owner check.
-        Preconditions.checkCallAuthorization(
-                isDefaultDeviceOwner(caller) || isCallerDelegate(caller, DELEGATION_CERT_INSTALL));
+        Preconditions.checkCallAuthorization(hasDeviceIdAccessUnchecked(caller.getPackageName(),
+                caller.getUid()));
     }
 
     @VisibleForTesting
@@ -5856,7 +5835,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
         final boolean deviceIdAttestationRequired = attestationUtilsFlags != null;
         KeyGenParameterSpec keySpec = parcelableKeySpec.getSpec();
         final String alias = keySpec.getKeystoreAlias();
-
         Preconditions.checkStringNotEmpty(alias, "Empty alias provided");
         Preconditions.checkArgument(
                 !deviceIdAttestationRequired || keySpec.getAttestationChallenge() != null,
@@ -9389,26 +9367,33 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
         if (!hasPermission(permission.READ_PHONE_STATE, pid, uid)) {
             return false;
         }
+        return hasDeviceIdAccessUnchecked(packageName, uid);
+    }
 
-        // Allow access to the device owner or delegate cert installer or profile owner of an
-        // affiliated user
+    /**
+     * 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.
+     */
+    private boolean hasDeviceIdAccessUnchecked(String packageName, int uid) {
+        // Is the caller a  device owner, delegate cert installer or profile owner of an
+        // affiliated user.
         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);
-        // Allow access to the profile owner for the specified user, or delegate cert installer
-        // But only if this is an organization-owned device.
+        // 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)
-                        || isCallerDelegate(packageName, uid, DELEGATION_CERT_INSTALL));
+                || isCallerDelegate(packageName, uid, DELEGATION_CERT_INSTALL));
         if (isCallerProfileOwnerOrDelegate && (isProfileOwnerOfOrganizationOwnedDevice(userId)
                 || isUserAffiliatedWithDevice(userId))) {
             return true;
         }
-
         return false;
     }