From d7eb72b9ff4948bdf9fc89badb267c0e93cfbc06 Mon Sep 17 00:00:00 2001 From: Sergey Nikolaienkov Date: Tue, 7 Dec 2021 12:19:14 +0100 Subject: [PATCH] Permissions checks for CDM.associate() Bug: 209581046 Bug: 208307075 Test: atest CompanionDeviceManagerTest Change-Id: Ieb3ae096ed6e87cabf5a86db4cf2b92308e50426 --- .../AssociationRequestsProcessor.java | 4 +- .../CompanionDeviceManagerService.java | 18 ++++--- .../server/companion/PermissionsUtils.java | 50 +++++++++++++++---- 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/services/companion/java/com/android/server/companion/AssociationRequestsProcessor.java b/services/companion/java/com/android/server/companion/AssociationRequestsProcessor.java index a0a00f7629b1b..bcc345ff98e4c 100644 --- 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 diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index e0cd4725bce27..b32d5434558b4 100644 --- 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 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 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); } diff --git a/services/companion/java/com/android/server/companion/PermissionsUtils.java b/services/companion/java/com/android/server/companion/PermissionsUtils.java index 45097f0d0ea6b..ea570895c18f1 100644 --- 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: + *
    + *
  • the package itself + *
  • the System ({@link android.os.Process#SYSTEM_UID}) + *
  • holds {@link Manifest.permission#MANAGE_COMPANION_DEVICES} and, if belongs to a + * different user, also holds {@link Manifest.permission#INTERACT_ACROSS_USERS}. + *
+ * @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) {