From 7f7195bf2ce4ae2007077846d3a80ad7652a58e8 Mon Sep 17 00:00:00 2001 From: Alex Kershaw Date: Mon, 10 Feb 2020 22:51:53 +0000 Subject: [PATCH] Log cross-profile metrics Log metrics for the new app-op permission INTERACT_ACROSS_PROFILES and each of the new app-facing and DPC-facing APIs where possible. Simple getters do not have logging. Setters have logging but only when the value has changed. I always moved the logging into a separate private method. This is done to keep the logging code from overpowering the actual logic in the corresponding methods, particularly when there are conditions attached to when we want to log. There are also a few minor clean-ups in CrossProfileAppsServiceImpl. Bug: 136249261 Bug: 149370554 Bug: 149318411 Bug: 149370875 Bug: 149370515 Test: atest com.android.cts.devicepolicy.CrossProfileAppsHostSideTest Test: atest com.android.cts.devicepolicy.CrossProfileAppsPermissionHostSideTest Test: atest com.android.cts.devicepolicy.ManagedProfileCrossProfileTest Test: atest com.android.cts.devicepolicy.ManagedProfileTest Change-Id: Ibf2899f9b9974387ed1ba62fd02ece54a4c1564b --- .../pm/CrossProfileAppsServiceImpl.java | 77 ++++++++++++++----- .../DevicePolicyManagerService.java | 12 +++ 2 files changed, 71 insertions(+), 18 deletions(-) diff --git a/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java b/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java index 96d3cc1f5f64b..3939f2642bc81 100644 --- a/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java +++ b/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java @@ -68,7 +68,6 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { private Context mContext; private Injector mInjector; - private AppOpsManager mAppOpsManager; public CrossProfileAppsServiceImpl(Context context) { this(context, new InjectorImpl(context)); @@ -217,19 +216,27 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { mInjector.getActivityTaskManagerInternal().startActivityAsUser(caller, callingPackage, callingFeatureId, launchIntent, /* options= */ null, userId); + logStartActivityByIntent(callingPackage); + } + + private void logStartActivityByIntent(String packageName) { + DevicePolicyEventLogger + .createEvent(DevicePolicyEnums.START_ACTIVITY_BY_INTENT) + .setStrings(packageName) + .setBoolean(isCallingUserAManagedProfile()) + .write(); } @Override public boolean canRequestInteractAcrossProfiles(String callingPackage) { Objects.requireNonNull(callingPackage); verifyCallingPackage(callingPackage); - return canRequestInteractAcrossProfilesUnchecked( - callingPackage, mInjector.getCallingUserId()); + return canRequestInteractAcrossProfilesUnchecked(callingPackage); } - private boolean canRequestInteractAcrossProfilesUnchecked( - String packageName, @UserIdInt int userId) { - List targetUserProfiles = getTargetUserProfilesUnchecked(packageName, userId); + private boolean canRequestInteractAcrossProfilesUnchecked(String packageName) { + List targetUserProfiles = + getTargetUserProfilesUnchecked(packageName, mInjector.getCallingUserId()); if (targetUserProfiles.isEmpty()) { return false; } @@ -256,12 +263,11 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { Objects.requireNonNull(callingPackage); verifyCallingPackage(callingPackage); - final List targetUserProfiles = getTargetUserProfilesUnchecked(callingPackage, - mInjector.getCallingUserId()); + final List targetUserProfiles = getTargetUserProfilesUnchecked( + callingPackage, mInjector.getCallingUserId()); if (targetUserProfiles.isEmpty()) { return false; } - return hasInteractAcrossProfilesPermission(callingPackage); } @@ -363,6 +369,12 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { }); } + /** + * See {@link android.content.pm.CrossProfileApps#setInteractAcrossProfilesAppOp(String, int)}. + * + *

Logs metrics. Use {@link #setInteractAcrossProfilesAppOpUnchecked(String, int, boolean)} + * to avoid permission checks or to specify not to log metrics. + */ @Override public void setInteractAcrossProfilesAppOp(String packageName, @Mode int newMode) { final int callingUid = mInjector.getCallingUid(); @@ -379,6 +391,11 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { "MANAGE_APP_OPS_MODES or CONFIGURE_INTERACT_ACROSS_PROFILES is required to set" + " the app-op for interacting across profiles."); } + setInteractAcrossProfilesAppOpUnchecked(packageName, newMode, /* logMetrics= */ true); + } + + private void setInteractAcrossProfilesAppOpUnchecked( + String packageName, @Mode int newMode, boolean logMetrics) { if (newMode == AppOpsManager.MODE_ALLOWED && !canConfigureInteractAcrossProfiles(packageName)) { // The user should not be prompted for apps that cannot request to interact across @@ -394,7 +411,7 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { if (!isPackageInstalled(packageName, profileId)) { continue; } - setInteractAcrossProfilesAppOpForUser(packageName, newMode, profileId); + setInteractAcrossProfilesAppOpForUser(packageName, newMode, profileId, logMetrics); } } @@ -413,16 +430,16 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { } private void setInteractAcrossProfilesAppOpForUser( - String packageName, @Mode int newMode, @UserIdInt int userId) { + String packageName, @Mode int newMode, @UserIdInt int userId, boolean logMetrics) { try { - setInteractAcrossProfilesAppOpForUserOrThrow(packageName, newMode, userId); + setInteractAcrossProfilesAppOpForUserOrThrow(packageName, newMode, userId, logMetrics); } catch (PackageManager.NameNotFoundException e) { Slog.e(TAG, "Missing package " + packageName + " on user ID " + userId, e); } } private void setInteractAcrossProfilesAppOpForUserOrThrow( - String packageName, @Mode int newMode, @UserIdInt int userId) + String packageName, @Mode int newMode, @UserIdInt int userId, boolean logMetrics) throws PackageManager.NameNotFoundException { final int uid = mInjector.getPackageManager() .getPackageUidAsUser(packageName, /* flags= */ 0, userId); @@ -444,6 +461,28 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { .setMode(OP_INTERACT_ACROSS_PROFILES, uid, packageName, newMode); } sendCanInteractAcrossProfilesChangedBroadcast(packageName, uid, UserHandle.of(userId)); + maybeLogSetInteractAcrossProfilesAppOp(packageName, newMode, userId, logMetrics, uid); + } + + private void maybeLogSetInteractAcrossProfilesAppOp( + String packageName, + @Mode int newMode, + @UserIdInt int userId, + boolean logMetrics, + int uid) { + if (!logMetrics) { + return; + } + if (userId != mInjector.getCallingUserId()) { + // Only log once per profile group by checking for the calling user ID. + return; + } + DevicePolicyEventLogger + .createEvent(DevicePolicyEnums.SET_INTERACT_ACROSS_PROFILES_APP_OP) + .setStrings(packageName) + .setInt(newMode) + .setBoolean(appDeclaresCrossProfileAttribute(uid)) + .write(); } /** @@ -541,11 +580,13 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { permission, uid, /* owningUid= */-1, /* exported= */ true); } - private AppOpsManager getAppOpsManager() { - if (mAppOpsManager == null) { - mAppOpsManager = mContext.getSystemService(AppOpsManager.class); - } - return mAppOpsManager; + private boolean isCallingUserAManagedProfile() { + return isManagedProfile(mInjector.getCallingUserId()); + } + + private boolean isManagedProfile(@UserIdInt int userId) { + return mInjector.withCleanCallingIdentity(() + -> mContext.getSystemService(UserManager.class).isManagedProfile(userId)); } private static class InjectorImpl implements Injector { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index aa5dafce0450d..063ad0fb50da0 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -15230,15 +15230,27 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { final ActiveAdmin admin = getActiveAdminForCallerLocked(who, DeviceAdminInfo.USES_POLICY_PROFILE_OWNER); previousCrossProfilePackages = admin.mCrossProfilePackages; + if (packageNames.equals(previousCrossProfilePackages)) { + return; + } admin.mCrossProfilePackages = packageNames; saveSettingsLocked(mInjector.userHandleGetCallingUserId()); } + logSetCrossProfilePackages(who, packageNames); final CrossProfileApps crossProfileApps = mContext.getSystemService(CrossProfileApps.class); mInjector.binderWithCleanCallingIdentity( () -> crossProfileApps.resetInteractAcrossProfilesAppOps( previousCrossProfilePackages, new HashSet<>(packageNames))); } + private void logSetCrossProfilePackages(ComponentName who, List packageNames) { + DevicePolicyEventLogger + .createEvent(DevicePolicyEnums.SET_CROSS_PROFILE_PACKAGES) + .setAdmin(who) + .setStrings(packageNames.toArray(new String[packageNames.size()])) + .write(); + } + @Override public List getCrossProfilePackages(ComponentName who) { if (!mHasFeature) {