Permissions checks for CDM.associate()

Bug: 209581046 Bug: 208307075 Test: atest CompanionDeviceManagerTest Change-Id: Ieb3ae096ed6e87cabf5a86db4cf2b92308e50426
2021-12-07 12:19:14 +01:00
parent f7d3ea8718
commit d7eb72b9ff
3 changed files with 51 additions and 21 deletions
--- a/services/companion/java/com/android/server/companion/AssociationRequestsProcessor.java
+++ b/services/companion/java/com/android/server/companion/AssociationRequestsProcessor.java
@@ -20,7 +20,7 @@ import static com.android.internal.util.CollectionUtils.filter;
 import static com.android.internal.util.FunctionalUtils.uncheckExceptions;
 import static com.android.server.companion.CompanionDeviceManagerService.DEBUG;
 import static com.android.server.companion.CompanionDeviceManagerService.LOG_TAG;
-import static com.android.server.companion.PermissionsUtils.enforceCallerPermissionsToRequest;
+import static com.android.server.companion.PermissionsUtils.enforcePermissionsForAssociation;
 import static com.android.server.companion.RolesUtils.isRoleHolder;

 import static java.util.Objects.requireNonNull;
@@ -110,7 +110,7 @@ class AssociationRequestsProcessor {
        }

        // 1. Enforce permissions and other requirements.
-        enforceCallerPermissionsToRequest(mContext, request, packageName, userId);
+        enforcePermissionsForAssociation(mContext, request, packageName, userId);
        mService.checkUsesFeature(packageName, userId);

        // 2. Check if association can be created without launching UI (i.e. CDM needs NEITHER
--- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java
+++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java
@@ -39,7 +39,8 @@ import static com.android.internal.util.function.pooled.PooledLambda.obtainRunna
 import static com.android.server.companion.PermissionsUtils.checkCallerCanManageAssociationsForPackage;
 import static com.android.server.companion.PermissionsUtils.checkCallerCanManageCompanionDevice;
 import static com.android.server.companion.PermissionsUtils.enforceCallerCanInteractWithUserId;
-import static com.android.server.companion.PermissionsUtils.enforceCallerCanManagerCompanionDevice;
+import static com.android.server.companion.PermissionsUtils.enforceCallerCanManageAssociationsForPackage;
+import static com.android.server.companion.PermissionsUtils.enforceCallerCanManageCompanionDevice;
 import static com.android.server.companion.PermissionsUtils.enforceCallerIsSystemOr;
 import static com.android.server.companion.RolesUtils.addRoleHolderForAssociation;
 import static com.android.server.companion.RolesUtils.removeRoleHolderForAssociation;
@@ -401,15 +402,16 @@ public class CompanionDeviceManagerService extends SystemService {
            Slog.i(LOG_TAG, "associate() "
                    + "request=" + request + ", "
                    + "package=u" + userId + "/" + packageName);
+            enforceCallerCanManageAssociationsForPackage(getContext(), userId, packageName,
+                    "create associations");
+
            mAssociationRequestsProcessor.process(request, packageName, userId, callback);
        }

        @Override
        public List<AssociationInfo> getAssociations(String packageName, int userId) {
-            if (!checkCallerCanManageAssociationsForPackage(getContext(), userId, packageName)) {
-                throw new SecurityException("Caller (uid=" + getCallingUid() + ") does not have "
-                        + "permissions to get associations for u" + userId + "/" + packageName);
-            }
+            enforceCallerCanManageAssociationsForPackage(getContext(), userId, packageName,
+                    "get associations");

            if (!checkCallerCanManageCompanionDevice(getContext())) {
                // If the caller neither is system nor holds MANAGE_COMPANION_DEVICES: it needs to
@@ -424,7 +426,7 @@ public class CompanionDeviceManagerService extends SystemService {
        @Override
        public List<AssociationInfo> getAllAssociationsForUser(int userId) throws RemoteException {
            enforceCallerCanInteractWithUserId(getContext(), userId);
-            enforceCallerCanManagerCompanionDevice(getContext(), "getAllAssociationsForUser");
+            enforceCallerCanManageCompanionDevice(getContext(), "getAllAssociationsForUser");

            return new ArrayList<>(
                    CompanionDeviceManagerService.this.getAllAssociationsForUser(userId));
@@ -434,7 +436,7 @@ public class CompanionDeviceManagerService extends SystemService {
        public void addOnAssociationsChangedListener(IOnAssociationsChangedListener listener,
                int userId) {
            enforceCallerCanInteractWithUserId(getContext(), userId);
-            enforceCallerCanManagerCompanionDevice(getContext(),
+            enforceCallerCanManageCompanionDevice(getContext(),
                    "addOnAssociationsChangedListener");

            //TODO: Implement.
@@ -621,7 +623,7 @@ public class CompanionDeviceManagerService extends SystemService {
        public void onShellCommand(FileDescriptor in, FileDescriptor out, FileDescriptor err,
                String[] args, ShellCallback callback, ResultReceiver resultReceiver)
                throws RemoteException {
-            enforceCallerCanManagerCompanionDevice(getContext(), "onShellCommand");
+            enforceCallerCanManageCompanionDevice(getContext(), "onShellCommand");
            new CompanionDeviceShellCommand(CompanionDeviceManagerService.this)
                    .exec(this, in, out, err, args, callback, resultReceiver);
        }
--- a/services/companion/java/com/android/server/companion/PermissionsUtils.java
+++ b/services/companion/java/com/android/server/companion/PermissionsUtils.java
@@ -24,6 +24,7 @@ import static android.companion.AssociationRequest.DEVICE_PROFILE_APP_STREAMING;
 import static android.companion.AssociationRequest.DEVICE_PROFILE_AUTOMOTIVE_PROJECTION;
 import static android.companion.AssociationRequest.DEVICE_PROFILE_WATCH;
 import static android.content.pm.PackageManager.PERMISSION_GRANTED;
+import static android.os.Binder.getCallingPid;
 import static android.os.Binder.getCallingUid;
 import static android.os.Process.SYSTEM_UID;
 import static android.os.UserHandle.getCallingUserId;
@@ -37,11 +38,13 @@ import android.annotation.UserIdInt;
 import android.companion.AssociationRequest;
 import android.companion.CompanionDeviceManager;
 import android.content.Context;
+import android.content.pm.PackageManagerInternal;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.util.ArrayMap;

 import com.android.internal.app.IAppOpsService;
+import com.android.server.LocalServices;

 import java.util.Map;

@@ -65,21 +68,19 @@ final class PermissionsUtils {
        DEVICE_PROFILE_TO_PERMISSION = unmodifiableMap(map);
    }

-    static void enforceCallerPermissionsToRequest(@NonNull Context context,
+    static void enforcePermissionsForAssociation(@NonNull Context context,
            @NonNull AssociationRequest request, @NonNull String packageName,
            @UserIdInt int userId) {
-        enforceCallerCanInteractWithUserId(context, userId);
-        enforceCallerIsSystemOr(userId, packageName);
-
-        enforceRequestDeviceProfilePermissions(context, request.getDeviceProfile());
+        final int packageUid = getPackageUid(userId, packageName);
+        enforceRequestDeviceProfilePermissions(context, request.getDeviceProfile(), packageUid);

        if (request.isSelfManaged()) {
-            enforceRequestSelfManagedPermission(context);
+            enforceRequestSelfManagedPermission(context, packageUid);
        }
    }

    static void enforceRequestDeviceProfilePermissions(
-            @NonNull Context context, @Nullable String deviceProfile) {
+            @NonNull Context context, @Nullable String deviceProfile, int packageUid) {
        // Device profile can be null.
        if (deviceProfile == null) return;

@@ -100,14 +101,15 @@ final class PermissionsUtils {
        }

        final String permission = DEVICE_PROFILE_TO_PERMISSION.get(deviceProfile);
-        if (context.checkCallingOrSelfPermission(permission) != PERMISSION_GRANTED) {
+        if (context.checkPermission(permission, getCallingPid(), packageUid)
+                != PERMISSION_GRANTED) {
            throw new SecurityException("Application must hold " + permission + " to associate "
                    + "with a device with " + deviceProfile + " profile.");
        }
    }

-    static void enforceRequestSelfManagedPermission(@NonNull Context context) {
-        if (context.checkCallingOrSelfPermission(REQUEST_COMPANION_SELF_MANAGED)
+    static void enforceRequestSelfManagedPermission(@NonNull Context context, int packageUid) {
+        if (context.checkPermission(REQUEST_COMPANION_SELF_MANAGED, getCallingPid(), packageUid)
                != PERMISSION_GRANTED) {
            throw new SecurityException("Application does not hold "
                    + REQUEST_COMPANION_SELF_MANAGED);
@@ -159,13 +161,34 @@ final class PermissionsUtils {
        return context.checkCallingPermission(MANAGE_COMPANION_DEVICES) == PERMISSION_GRANTED;
    }

-    static void enforceCallerCanManagerCompanionDevice(@NonNull Context context,
+    static void enforceCallerCanManageCompanionDevice(@NonNull Context context,
            @Nullable String message) {
        if (getCallingUid() == SYSTEM_UID) return;

        context.enforceCallingPermission(MANAGE_COMPANION_DEVICES, message);
    }

+    static void enforceCallerCanManageAssociationsForPackage(@NonNull Context context,
+            @UserIdInt int userId, @NonNull String packageName,
+            @Nullable String actionDescription) {
+        if (checkCallerCanManageAssociationsForPackage(context, userId, packageName)) return;
+
+        throw new SecurityException("Caller (uid=" + getCallingUid() + ") does not have "
+                + "permissions to "
+                + (actionDescription != null ? actionDescription : "manage associations")
+                + " for u" + userId + "/" + packageName);
+    }
+
+    /**
+     * Check if the caller is either:
+     * <ul>
+     * <li> the package itself
+     * <li> the System ({@link android.os.Process#SYSTEM_UID})
+     * <li> holds {@link Manifest.permission#MANAGE_COMPANION_DEVICES} and, if belongs to a
+     * different user, also holds {@link Manifest.permission#INTERACT_ACROSS_USERS}.
+     * </ul>
+     * @return whether the caller is one of the above.
+     */
    static boolean checkCallerCanManageAssociationsForPackage(@NonNull Context context,
            @UserIdInt int userId, @NonNull String packageName) {
        if (checkCallerIsSystemOr(userId, packageName)) return true;
@@ -184,6 +207,11 @@ final class PermissionsUtils {
        }
    }

+    private static int getPackageUid(@UserIdInt int userId, @NonNull String packageName) {
+        return LocalServices.getService(PackageManagerInternal.class)
+                .getPackageUid(packageName, 0, userId);
+    }
+
    private static IAppOpsService getAppOpsService() {
        if (sAppOpsService == null) {
            synchronized (PermissionsUtils.class) {