From 640525c3ec700c9d2be8f1514ef02ae3a4ad0d93 Mon Sep 17 00:00:00 2001 From: Guojing Yuan Date: Wed, 31 May 2023 21:47:13 +0000 Subject: [PATCH] [CDM perm sync] Skip user consent for the same OEM devices Bug: 193583135 Test: manually tested the CDM test app and Pixel watch app Change-Id: Ic0633860856afa881ac78798acc079d99405438c --- .../AssociationRequestsProcessor.java | 68 +---------------- .../CompanionDeviceManagerService.java | 3 +- .../server/companion/PackageUtils.java | 76 ++++++++++++++++++- .../SystemDataTransferProcessor.java | 48 ++++++++---- 4 files changed, 110 insertions(+), 85 deletions(-) diff --git a/services/companion/java/com/android/server/companion/AssociationRequestsProcessor.java b/services/companion/java/com/android/server/companion/AssociationRequestsProcessor.java index 3e7d759d51140..631b4531b9b9e 100644 --- a/services/companion/java/com/android/server/companion/AssociationRequestsProcessor.java +++ b/services/companion/java/com/android/server/companion/AssociationRequestsProcessor.java @@ -47,7 +47,6 @@ import android.content.Context; import android.content.Intent; import android.content.IntentSender; import android.content.pm.PackageManagerInternal; -import android.content.pm.Signature; import android.net.MacAddress; import android.os.Binder; import android.os.Bundle; @@ -55,17 +54,11 @@ import android.os.Handler; import android.os.RemoteException; import android.os.ResultReceiver; import android.os.UserHandle; -import android.util.Log; -import android.util.PackageUtils; import android.util.Slog; import com.android.internal.R; -import com.android.internal.util.ArrayUtils; -import java.util.Arrays; -import java.util.HashSet; import java.util.List; -import java.util.Set; /** * Class responsible for handling incoming {@link AssociationRequest}s. @@ -449,31 +442,6 @@ class AssociationRequestsProcessor { }; private boolean mayAssociateWithoutPrompt(@NonNull String packageName, @UserIdInt int userId) { - // Below we check if the requesting package is allowlisted (usually by the OEM) for creating - // CDM associations without user confirmation (prompt). - // For this we'll check to config arrays: - // - com.android.internal.R.array.config_companionDevicePackages - // and - // - com.android.internal.R.array.config_companionDeviceCerts. - // Both arrays are expected to contain similar number of entries. - // config_companionDevicePackages contains package names of the allowlisted packages. - // config_companionDeviceCerts contains SHA256 digests of the signatures of the - // corresponding packages. - // If a package may be signed with one of several certificates, its package name would - // appear multiple times in the config_companionDevicePackages, with different entries - // (one for each of the valid signing certificates) at the corresponding positions in - // config_companionDeviceCerts. - final String[] allowlistedPackages = mContext.getResources() - .getStringArray(com.android.internal.R.array.config_companionDevicePackages); - if (!ArrayUtils.contains(allowlistedPackages, packageName)) { - if (DEBUG) { - Log.d(TAG, packageName + " is not allowlisted for creating associations " - + "without user confirmation (prompt)"); - Log.v(TAG, "Allowlisted packages=" + Arrays.toString(allowlistedPackages)); - } - return false; - } - // Throttle frequent associations final long now = System.currentTimeMillis(); final List associationForPackage = @@ -493,40 +461,6 @@ class AssociationRequestsProcessor { } } - final String[] allowlistedPackagesSignatureDigests = mContext.getResources() - .getStringArray(com.android.internal.R.array.config_companionDeviceCerts); - final Set allowlistedSignatureDigestsForRequestingPackage = new HashSet<>(); - for (int i = 0; i < allowlistedPackages.length; i++) { - if (allowlistedPackages[i].equals(packageName)) { - final String digest = allowlistedPackagesSignatureDigests[i].replaceAll(":", ""); - allowlistedSignatureDigestsForRequestingPackage.add(digest); - } - } - - final Signature[] requestingPackageSignatures = mPackageManager.getPackage(packageName) - .getSigningDetails().getSignatures(); - final String[] requestingPackageSignatureDigests = - PackageUtils.computeSignaturesSha256Digests(requestingPackageSignatures); - - boolean requestingPackageSignatureAllowlisted = false; - for (String signatureDigest : requestingPackageSignatureDigests) { - if (allowlistedSignatureDigestsForRequestingPackage.contains(signatureDigest)) { - requestingPackageSignatureAllowlisted = true; - break; - } - } - - if (!requestingPackageSignatureAllowlisted) { - Slog.w(TAG, "Certificate mismatch for allowlisted package " + packageName); - if (DEBUG) { - Log.d(TAG, " > allowlisted signatures for " + packageName + ": [" - + String.join(", ", allowlistedSignatureDigestsForRequestingPackage) - + "]"); - Log.d(TAG, " > actual signatures for " + packageName + ": " - + Arrays.toString(requestingPackageSignatureDigests)); - } - } - - return requestingPackageSignatureAllowlisted; + return PackageUtils.isPackageAllowlisted(mContext, mPackageManager, packageName); } } diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index 89e8a1825d3e8..4d4328dd14e8a 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -247,7 +247,8 @@ public class CompanionDeviceManagerService extends SystemService { mCompanionAppController = new CompanionApplicationController( context, mAssociationStore, mDevicePresenceMonitor); mTransportManager = new CompanionTransportManager(context, mAssociationStore); - mSystemDataTransferProcessor = new SystemDataTransferProcessor(this, mAssociationStore, + mSystemDataTransferProcessor = new SystemDataTransferProcessor(this, + mPackageManagerInternal, mAssociationStore, mSystemDataTransferRequestStore, mTransportManager); // TODO(b/279663946): move context sync to a dedicated system service mCrossDeviceSyncController = new CrossDeviceSyncController(getContext(), mTransportManager); diff --git a/services/companion/java/com/android/server/companion/PackageUtils.java b/services/companion/java/com/android/server/companion/PackageUtils.java index 3ab4aa89406b4..db40fc495caba 100644 --- a/services/companion/java/com/android/server/companion/PackageUtils.java +++ b/services/companion/java/com/android/server/companion/PackageUtils.java @@ -20,6 +20,7 @@ import static android.content.pm.PackageManager.FEATURE_COMPANION_DEVICE_SETUP; import static android.content.pm.PackageManager.GET_CONFIGURATIONS; import static android.content.pm.PackageManager.GET_PERMISSIONS; +import static com.android.server.companion.CompanionDeviceManagerService.DEBUG; import static com.android.server.companion.CompanionDeviceManagerService.TAG; import android.Manifest; @@ -35,20 +36,28 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManager.PackageInfoFlags; import android.content.pm.PackageManager.ResolveInfoFlags; +import android.content.pm.PackageManagerInternal; import android.content.pm.ResolveInfo; import android.content.pm.ServiceInfo; +import android.content.pm.Signature; import android.os.Binder; +import android.util.Log; import android.util.Slog; +import com.android.internal.util.ArrayUtils; + import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; /** * Utility methods for working with {@link PackageInfo}-s. */ -final class PackageUtils { +public final class PackageUtils { private static final Intent COMPANION_SERVICE_INTENT = new Intent(CompanionDeviceService.SERVICE_INTERFACE); private static final String PROPERTY_PRIMARY_TAG = @@ -141,4 +150,69 @@ final class PackageUtils { return false; } } + + /** + * Check if the package is allowlisted in the overlay config. + * For this we'll check to config arrays: + * - com.android.internal.R.array.config_companionDevicePackages + * and + * - com.android.internal.R.array.config_companionDeviceCerts. + * Both arrays are expected to contain similar number of entries. + * config_companionDevicePackages contains package names of the allowlisted packages. + * config_companionDeviceCerts contains SHA256 digests of the signatures of the + * corresponding packages. + * If a package is signed with one of several certificates, its package name would + * appear multiple times in the config_companionDevicePackages, with different entries + * (one for each of the valid signing certificates) at the corresponding positions in + * config_companionDeviceCerts. + */ + public static boolean isPackageAllowlisted(Context context, + PackageManagerInternal packageManagerInternal, @NonNull String packageName) { + final String[] allowlistedPackages = context.getResources() + .getStringArray(com.android.internal.R.array.config_companionDevicePackages); + if (!ArrayUtils.contains(allowlistedPackages, packageName)) { + if (DEBUG) { + Log.d(TAG, packageName + " is not allowlisted."); + } + return false; + } + + final String[] allowlistedPackagesSignatureDigests = context.getResources() + .getStringArray(com.android.internal.R.array.config_companionDeviceCerts); + final Set allowlistedSignatureDigestsForRequestingPackage = new HashSet<>(); + for (int i = 0; i < allowlistedPackages.length; i++) { + if (allowlistedPackages[i].equals(packageName)) { + final String digest = allowlistedPackagesSignatureDigests[i].replaceAll(":", ""); + allowlistedSignatureDigestsForRequestingPackage.add(digest); + } + } + + final Signature[] requestingPackageSignatures = packageManagerInternal.getPackage( + packageName) + .getSigningDetails().getSignatures(); + final String[] requestingPackageSignatureDigests = + android.util.PackageUtils.computeSignaturesSha256Digests( + requestingPackageSignatures); + + boolean requestingPackageSignatureAllowlisted = false; + for (String signatureDigest : requestingPackageSignatureDigests) { + if (allowlistedSignatureDigestsForRequestingPackage.contains(signatureDigest)) { + requestingPackageSignatureAllowlisted = true; + break; + } + } + + if (!requestingPackageSignatureAllowlisted) { + Slog.w(TAG, "Certificate mismatch for allowlisted package " + packageName); + if (DEBUG) { + Log.d(TAG, " > allowlisted signatures for " + packageName + ": [" + + String.join(", ", allowlistedSignatureDigestsForRequestingPackage) + + "]"); + Log.d(TAG, " > actual signatures for " + packageName + ": " + + Arrays.toString(requestingPackageSignatureDigests)); + } + } + + return requestingPackageSignatureAllowlisted; + } } 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 82387a182f08d..7af4957213ee9 100644 --- a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java +++ b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java @@ -37,6 +37,7 @@ import android.companion.datatransfer.SystemDataTransferRequest; import android.content.ComponentName; import android.content.Context; import android.content.Intent; +import android.content.pm.PackageManagerInternal; import android.os.Binder; import android.os.Bundle; import android.os.Handler; @@ -50,6 +51,7 @@ import android.util.Slog; import com.android.internal.R; import com.android.server.companion.AssociationStore; import com.android.server.companion.CompanionDeviceManagerService; +import com.android.server.companion.PackageUtils; import com.android.server.companion.PermissionsUtils; import com.android.server.companion.transport.CompanionTransportManager; @@ -77,6 +79,7 @@ public class SystemDataTransferProcessor { "system_data_transfer_result_receiver"; private final Context mContext; + private final PackageManagerInternal mPackageManager; private final AssociationStore mAssociationStore; private final SystemDataTransferRequestStore mSystemDataTransferRequestStore; private final CompanionTransportManager mTransportManager; @@ -85,10 +88,12 @@ public class SystemDataTransferProcessor { private final ComponentName mCompanionDeviceDataTransferActivity; public SystemDataTransferProcessor(CompanionDeviceManagerService service, + PackageManagerInternal packageManager, AssociationStore associationStore, SystemDataTransferRequestStore systemDataTransferRequestStore, CompanionTransportManager transportManager) { mContext = service.getContext(); + mPackageManager = packageManager; mAssociationStore = associationStore; mSystemDataTransferRequestStore = systemDataTransferRequestStore; mTransportManager = transportManager; @@ -132,6 +137,11 @@ public class SystemDataTransferProcessor { */ public PendingIntent buildPermissionTransferUserConsentIntent(String packageName, @UserIdInt int userId, int associationId) { + if (PackageUtils.isPackageAllowlisted(mContext, mPackageManager, packageName)) { + Slog.i(LOG_TAG, "User consent Intent should be skipped. Returning null."); + return null; + } + final AssociationInfo association = resolveAssociation(packageName, userId, associationId); Slog.i(LOG_TAG, "Creating permission sync intent for userId [" + userId @@ -175,23 +185,29 @@ public class SystemDataTransferProcessor { final AssociationInfo association = resolveAssociation(packageName, userId, associationId); // Check if the request has been consented by the user. - List storedRequests = - mSystemDataTransferRequestStore.readRequestsByAssociationId(userId, - associationId); - boolean hasConsented = false; - for (SystemDataTransferRequest storedRequest : storedRequests) { - if (storedRequest instanceof PermissionSyncRequest && storedRequest.isUserConsented()) { - hasConsented = true; - break; + if (PackageUtils.isPackageAllowlisted(mContext, mPackageManager, packageName)) { + Slog.i(LOG_TAG, "Skip user consent check due to the same OEM package."); + } else { + List storedRequests = + mSystemDataTransferRequestStore.readRequestsByAssociationId(userId, + associationId); + boolean hasConsented = false; + for (SystemDataTransferRequest storedRequest : storedRequests) { + if (storedRequest instanceof PermissionSyncRequest + && storedRequest.isUserConsented()) { + hasConsented = true; + break; + } + } + if (!hasConsented) { + String message = "User " + userId + " hasn't consented permission sync."; + Slog.e(LOG_TAG, message); + try { + callback.onError(message); + } catch (RemoteException ignored) { + } + return; } - } - if (!hasConsented) { - String message = "User " + userId + " hasn't consented permission sync."; - Slog.e(LOG_TAG, message); - try { - callback.onError(message); - } catch (RemoteException ignored) { } - return; } // Start permission sync