diff options
6 files changed, 69 insertions, 44 deletions
diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index 0d0c21dcf49c..30e4a3eb77e6 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -34,6 +34,7 @@ import static com.android.internal.util.function.pooled.PooledLambda.obtainMessa import static com.android.server.companion.utils.PackageUtils.enforceUsesCompanionDeviceFeature; import static com.android.server.companion.utils.PackageUtils.getPackageInfo; import static com.android.server.companion.utils.PackageUtils.isRestrictedSettingsAllowed; +import static com.android.server.companion.utils.PermissionsUtils.checkCallerCanManageCompanionDevice; import static com.android.server.companion.utils.PermissionsUtils.enforceCallerCanManageAssociationsForPackage; import static com.android.server.companion.utils.PermissionsUtils.enforceCallerIsSystemOr; import static com.android.server.companion.utils.PermissionsUtils.enforceCallerIsSystemOrCanInteractWithUserId; @@ -334,6 +335,12 @@ public class CompanionDeviceManagerService extends SystemService { enforceCallerCanManageAssociationsForPackage(getContext(), userId, packageName, "get associations"); + if (!checkCallerCanManageCompanionDevice(getContext())) { + // If the caller neither is system nor holds MANAGE_COMPANION_DEVICES: it needs to + // request the feature (also: the caller is the app itself). + enforceUsesCompanionDeviceFeature(getContext(), userId, packageName); + } + return mAssociationStore.getActiveAssociationsByPackage(userId, packageName); } diff --git a/services/companion/java/com/android/server/companion/association/AssociationRequestsProcessor.java b/services/companion/java/com/android/server/companion/association/AssociationRequestsProcessor.java index 3fbd8560b82c..d09d7e672f9d 100644 --- a/services/companion/java/com/android/server/companion/association/AssociationRequestsProcessor.java +++ b/services/companion/java/com/android/server/companion/association/AssociationRequestsProcessor.java @@ -347,8 +347,6 @@ public class AssociationRequestsProcessor { * Set association tag. */ public void setAssociationTag(int associationId, String tag) { - Slog.i(TAG, "Setting association tag=[" + tag + "] to id=[" + associationId + "]..."); - AssociationInfo association = mAssociationStore.getAssociationWithCallerChecks( associationId); association = (new AssociationInfo.Builder(association)).setTag(tag).build(); diff --git a/services/companion/java/com/android/server/companion/association/AssociationStore.java b/services/companion/java/com/android/server/companion/association/AssociationStore.java index 757abd927ac8..29e8095f8680 100644 --- a/services/companion/java/com/android/server/companion/association/AssociationStore.java +++ b/services/companion/java/com/android/server/companion/association/AssociationStore.java @@ -18,7 +18,7 @@ package com.android.server.companion.association; import static com.android.server.companion.utils.MetricUtils.logCreateAssociation; import static com.android.server.companion.utils.MetricUtils.logRemoveAssociation; -import static com.android.server.companion.utils.PermissionsUtils.enforceCallerCanManageAssociationsForPackage; +import static com.android.server.companion.utils.PermissionsUtils.checkCallerCanManageAssociationsForPackage; import android.annotation.IntDef; import android.annotation.NonNull; @@ -457,10 +457,6 @@ public class AssociationStore { /** * Get association by id with caller checks. - * - * If the association is not found, an IllegalArgumentException would be thrown. - * - * If the caller can't access the association, a SecurityException would be thrown. */ @NonNull public AssociationInfo getAssociationWithCallerChecks(int associationId) { @@ -470,9 +466,13 @@ public class AssociationStore { "getAssociationWithCallerChecks() Association id=[" + associationId + "] doesn't exist."); } - enforceCallerCanManageAssociationsForPackage(mContext, association.getUserId(), - association.getPackageName(), null); - return association; + if (checkCallerCanManageAssociationsForPackage(mContext, association.getUserId(), + association.getPackageName())) { + return association; + } + + throw new IllegalArgumentException( + "The caller can't interact with the association id=[" + associationId + "]."); } /** diff --git a/services/companion/java/com/android/server/companion/association/DisassociationProcessor.java b/services/companion/java/com/android/server/companion/association/DisassociationProcessor.java index 6f0baef019b3..8c1116b7a612 100644 --- a/services/companion/java/com/android/server/companion/association/DisassociationProcessor.java +++ b/services/companion/java/com/android/server/companion/association/DisassociationProcessor.java @@ -98,6 +98,7 @@ public class DisassociationProcessor { Slog.i(TAG, "Disassociating id=[" + id + "]..."); final AssociationInfo association = mAssociationStore.getAssociationWithCallerChecks(id); + final int userId = association.getUserId(); final String packageName = association.getPackageName(); final String deviceProfile = association.getDeviceProfile(); diff --git a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java index 00e049c1e1ff..026d29c9f821 100644 --- a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java +++ b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java @@ -122,6 +122,7 @@ public class SystemDataTransferProcessor { */ public boolean isPermissionTransferUserConsented(int associationId) { mAssociationStore.getAssociationWithCallerChecks(associationId); + PermissionSyncRequest request = getPermissionSyncRequest(associationId); if (request == null) { return false; @@ -146,12 +147,12 @@ public class SystemDataTransferProcessor { return null; } - Slog.i(LOG_TAG, "Creating permission sync intent for userId [" + userId - + "] associationId [" + associationId + "]"); - final AssociationInfo association = mAssociationStore.getAssociationWithCallerChecks( associationId); + Slog.i(LOG_TAG, "Creating permission sync intent for userId [" + userId + + "] associationId [" + associationId + "]"); + // Create an internal intent to launch the user consent dialog final Bundle extras = new Bundle(); PermissionSyncRequest request = new PermissionSyncRequest(associationId); @@ -219,9 +220,7 @@ public class SystemDataTransferProcessor { * Enable perm sync for the association */ public void enablePermissionsSync(int associationId) { - AssociationInfo association = mAssociationStore.getAssociationWithCallerChecks( - associationId); - int userId = association.getUserId(); + int userId = mAssociationStore.getAssociationWithCallerChecks(associationId).getUserId(); PermissionSyncRequest request = new PermissionSyncRequest(associationId); request.setUserConsented(true); mSystemDataTransferRequestStore.writeRequest(userId, request); @@ -231,9 +230,7 @@ public class SystemDataTransferProcessor { * Disable perm sync for the association */ public void disablePermissionsSync(int associationId) { - AssociationInfo association = mAssociationStore.getAssociationWithCallerChecks( - associationId); - int userId = association.getUserId(); + int userId = mAssociationStore.getAssociationWithCallerChecks(associationId).getUserId(); PermissionSyncRequest request = new PermissionSyncRequest(associationId); request.setUserConsented(false); mSystemDataTransferRequestStore.writeRequest(userId, request); @@ -244,9 +241,8 @@ public class SystemDataTransferProcessor { */ @Nullable public PermissionSyncRequest getPermissionSyncRequest(int associationId) { - AssociationInfo association = mAssociationStore.getAssociationWithCallerChecks( - associationId); - int userId = association.getUserId(); + int userId = mAssociationStore.getAssociationWithCallerChecks(associationId) + .getUserId(); List<SystemDataTransferRequest> requests = mSystemDataTransferRequestStore.readRequestsByAssociationId(userId, associationId); @@ -263,9 +259,7 @@ public class SystemDataTransferProcessor { */ public void removePermissionSyncRequest(int associationId) { Binder.withCleanCallingIdentity(() -> { - AssociationInfo association = mAssociationStore.getAssociationWithCallerChecks( - associationId); - int userId = association.getUserId(); + int userId = mAssociationStore.getAssociationById(associationId).getUserId(); mSystemDataTransferRequestStore.removeRequestsByAssociationId(userId, associationId); }); } diff --git a/services/companion/java/com/android/server/companion/utils/PermissionsUtils.java b/services/companion/java/com/android/server/companion/utils/PermissionsUtils.java index 796d2851760f..f397814f7ad7 100644 --- a/services/companion/java/com/android/server/companion/utils/PermissionsUtils.java +++ b/services/companion/java/com/android/server/companion/utils/PermissionsUtils.java @@ -149,6 +149,21 @@ public final class PermissionsUtils { } /** + * Check if the caller is system UID or the provided user. + */ + public static boolean checkCallerIsSystemOr(@UserIdInt int userId, + @NonNull String packageName) { + final int callingUid = getCallingUid(); + if (callingUid == SYSTEM_UID) return true; + + if (getCallingUserId() != userId) return false; + + if (!checkPackage(callingUid, packageName)) return false; + + return true; + } + + /** * Check if the calling user id matches the userId, and if the package belongs to * the calling uid. */ @@ -169,30 +184,21 @@ public final class PermissionsUtils { } /** + * Check if the caller holds the necessary permission to manage companion devices. + */ + public static boolean checkCallerCanManageCompanionDevice(@NonNull Context context) { + if (getCallingUid() == SYSTEM_UID) return true; + + return context.checkCallingPermission(MANAGE_COMPANION_DEVICES) == PERMISSION_GRANTED; + } + + /** * Require the caller to be able to manage the associations for the package. */ public static void enforceCallerCanManageAssociationsForPackage(@NonNull Context context, @UserIdInt int userId, @NonNull String packageName, @Nullable String actionDescription) { - final int callingUid = getCallingUid(); - - // If the caller is the system - if (callingUid == SYSTEM_UID) { - return; - } - - // If caller can manage the package or has the permissions to manage companion devices - boolean canInteractAcrossUsers = context.checkCallingPermission(INTERACT_ACROSS_USERS) - == PERMISSION_GRANTED; - boolean canManageCompanionDevices = context.checkCallingPermission(MANAGE_COMPANION_DEVICES) - == PERMISSION_GRANTED; - if (getCallingUserId() == userId) { - if (checkPackage(callingUid, packageName) || canManageCompanionDevices) { - return; - } - } else if (canInteractAcrossUsers && canManageCompanionDevices) { - return; - } + if (checkCallerCanManageAssociationsForPackage(context, userId, packageName)) return; throw new SecurityException("Caller (uid=" + getCallingUid() + ") does not have " + "permissions to " @@ -213,6 +219,25 @@ public final class PermissionsUtils { } } + /** + * 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. + */ + public static boolean checkCallerCanManageAssociationsForPackage(@NonNull Context context, + @UserIdInt int userId, @NonNull String packageName) { + if (checkCallerIsSystemOr(userId, packageName)) return true; + + if (!checkCallerCanInteractWithUserId(context, userId)) return false; + + return checkCallerCanManageCompanionDevice(context); + } + private static boolean checkPackage(@UserIdInt int uid, @NonNull String packageName) { try { return getAppOpsService().checkPackage(uid, packageName) == MODE_ALLOWED; |