From af6b08b09abb097298aafb048e8078ee01269448 Mon Sep 17 00:00:00 2001 From: Alex Johnston Date: Fri, 4 Mar 2022 10:40:42 +0000 Subject: [PATCH] Make screen capture per-device DO and COPE Test cases: - DO - Verify disabled for entire device - Verify still disabled after restart - Verify no longer disabled when DO is removed - COPE - parent - Verify disabled for entire device - Verify still disabled after restart - Verify no longer disabled when COPE PO is removed - not-parent - Verify disabled for user - Verify disabled after restart - Verify disabled when PO is removed Bug: 217558483 Test: DevicePolicyManagerTest OrgOwnedProfileOwnerTest#testScreenCaptureDisabled ScreenCaptureDisabledTest Change-Id: I977eb18619da46e1cfc5d0a8d351ea80d3ea7205 --- .../android/app/admin/DevicePolicyCache.java | 6 +- core/java/android/view/IWindowManager.aidl | 2 +- .../server/wm/ActivityTaskManagerService.java | 2 +- .../server/wm/RootWindowContainer.java | 4 +- .../server/wm/WindowManagerService.java | 6 +- .../com/android/server/wm/WindowState.java | 3 +- .../devicepolicy/DevicePolicyCacheImpl.java | 25 ++++-- .../DevicePolicyManagerService.java | 79 ++++++++++++------- .../devicepolicy/DevicePolicyManagerTest.java | 11 ++- 9 files changed, 87 insertions(+), 51 deletions(-) diff --git a/core/java/android/app/admin/DevicePolicyCache.java b/core/java/android/app/admin/DevicePolicyCache.java index 9c07f85a63903..da6237513562d 100644 --- a/core/java/android/app/admin/DevicePolicyCache.java +++ b/core/java/android/app/admin/DevicePolicyCache.java @@ -41,8 +41,7 @@ public abstract class DevicePolicyCache { /** * See {@link DevicePolicyManager#getScreenCaptureDisabled} */ - public abstract boolean isScreenCaptureAllowed(@UserIdInt int userHandle, - boolean ownerCanAddInternalSystemWindow); + public abstract boolean isScreenCaptureAllowed(@UserIdInt int userHandle); /** * Caches {@link DevicePolicyManager#getPasswordQuality(android.content.ComponentName)} of the @@ -70,8 +69,7 @@ public abstract class DevicePolicyCache { private static final EmptyDevicePolicyCache INSTANCE = new EmptyDevicePolicyCache(); @Override - public boolean isScreenCaptureAllowed(int userHandle, - boolean ownerCanAddInternalSystemWindow) { + public boolean isScreenCaptureAllowed(int userHandle) { return true; } diff --git a/core/java/android/view/IWindowManager.aidl b/core/java/android/view/IWindowManager.aidl index 5ce5477daa0f0..c83869c9ee686 100644 --- a/core/java/android/view/IWindowManager.aidl +++ b/core/java/android/view/IWindowManager.aidl @@ -249,7 +249,7 @@ interface IWindowManager * Set whether screen capture is disabled for all windows of a specific user from * the device policy cache. */ - void refreshScreenCaptureDisabled(int userId); + void refreshScreenCaptureDisabled(); // These can only be called with the SET_ORIENTATION permission. /** diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index d99770717a5dd..d254aaff1a1c6 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -3331,7 +3331,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { } userId = activity.mUserId; } - return DevicePolicyCache.getInstance().isScreenCaptureAllowed(userId, false); + return DevicePolicyCache.getInstance().isScreenCaptureAllowed(userId); } private void onLocalVoiceInteractionStartedLocked(IBinder activity, diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java index 22714c6f55937..0be29a9d0925c 100644 --- a/services/core/java/com/android/server/wm/RootWindowContainer.java +++ b/services/core/java/com/android/server/wm/RootWindowContainer.java @@ -643,9 +643,9 @@ class RootWindowContainer extends WindowContainer } } - void setSecureSurfaceState(int userId) { + void refreshSecureSurfaceState() { forAllWindows((w) -> { - if (w.mHasSurface && userId == w.mShowUserId) { + if (w.mHasSurface) { w.mWinAnimator.setSecureLocked(w.isSecureLocked()); } }, true /* traverseTopToBottom */); diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 2f4dc515fd117..c49cec238a31e 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -2009,15 +2009,15 @@ public class WindowManagerService extends IWindowManager.Stub * the device policy cache. */ @Override - public void refreshScreenCaptureDisabled(int userId) { + public void refreshScreenCaptureDisabled() { int callingUid = Binder.getCallingUid(); if (callingUid != SYSTEM_UID) { throw new SecurityException("Only system can call refreshScreenCaptureDisabled."); } synchronized (mGlobalLock) { - // Update secure surface for all windows belonging to this user. - mRoot.setSecureSurfaceState(userId); + // Refresh secure surface for all windows. + mRoot.refreshSecureSurfaceState(); } } diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 238f96ffd1e16..c6288a7da26a8 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -2040,8 +2040,7 @@ class WindowState extends WindowContainer implements WindowManagerP if ((mAttrs.flags & WindowManager.LayoutParams.FLAG_SECURE) != 0) { return true; } - return !DevicePolicyCache.getInstance().isScreenCaptureAllowed(mShowUserId, - mOwnerCanAddInternalSystemWindow); + return !DevicePolicyCache.getInstance().isScreenCaptureAllowed(mShowUserId); } /** diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyCacheImpl.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyCacheImpl.java index a3017990543f8..304d148252b1f 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyCacheImpl.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyCacheImpl.java @@ -18,6 +18,7 @@ package com.android.server.devicepolicy; import android.annotation.UserIdInt; import android.app.admin.DevicePolicyCache; import android.app.admin.DevicePolicyManager; +import android.os.UserHandle; import android.util.IndentingPrintWriter; import android.util.SparseBooleanArray; import android.util.SparseIntArray; @@ -37,8 +38,12 @@ public class DevicePolicyCacheImpl extends DevicePolicyCache { */ private final Object mLock = new Object(); + /** + * Indicates which user is screen capture disallowed on. Can be {@link UserHandle#USER_NULL}, + * {@link UserHandle#USER_ALL} or a concrete user ID. + */ @GuardedBy("mLock") - private final SparseBooleanArray mScreenCaptureDisabled = new SparseBooleanArray(); + private int mScreenCaptureDisallowedUser = UserHandle.USER_NULL; @GuardedBy("mLock") private final SparseIntArray mPasswordQuality = new SparseIntArray(); @@ -57,7 +62,6 @@ public class DevicePolicyCacheImpl extends DevicePolicyCache { public void onUserRemoved(int userHandle) { synchronized (mLock) { - mScreenCaptureDisabled.delete(userHandle); mPasswordQuality.delete(userHandle); mPermissionPolicy.delete(userHandle); mCanGrantSensorsPermissions.delete(userHandle); @@ -65,15 +69,22 @@ public class DevicePolicyCacheImpl extends DevicePolicyCache { } @Override - public boolean isScreenCaptureAllowed(int userHandle, boolean ownerCanAddInternalSystemWindow) { + public boolean isScreenCaptureAllowed(int userHandle) { synchronized (mLock) { - return !mScreenCaptureDisabled.get(userHandle) || ownerCanAddInternalSystemWindow; + return mScreenCaptureDisallowedUser != UserHandle.USER_ALL + && mScreenCaptureDisallowedUser != userHandle; } } - public void setScreenCaptureAllowed(int userHandle, boolean allowed) { + public int getScreenCaptureDisallowedUser() { synchronized (mLock) { - mScreenCaptureDisabled.put(userHandle, !allowed); + return mScreenCaptureDisallowedUser; + } + } + + public void setScreenCaptureDisallowedUser(int userHandle) { + synchronized (mLock) { + mScreenCaptureDisallowedUser = userHandle; } } @@ -125,7 +136,7 @@ public class DevicePolicyCacheImpl extends DevicePolicyCache { public void dump(IndentingPrintWriter pw) { pw.println("Device policy cache:"); pw.increaseIndent(); - pw.println("Screen capture disabled: " + mScreenCaptureDisabled.toString()); + pw.println("Screen capture disallowed user: " + mScreenCaptureDisallowedUser); pw.println("Password quality: " + mPasswordQuality.toString()); pw.println("Permission policy: " + mPermissionPolicy.toString()); pw.println("Admin can grant sensors permission: " diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 18bffebb427c4..c9f2f88a3b49e 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -1963,6 +1963,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { mOwners.removeProfileOwner(userHandle); mOwners.writeProfileOwner(userHandle); + pushScreenCapturePolicy(userHandle); DevicePolicyData policy = mUserData.get(userHandle); if (policy != null) { @@ -3183,8 +3184,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { @Override void handleStartUser(int userId) { - updateScreenCaptureDisabled(userId, - getScreenCaptureDisabled(null, userId, false)); + synchronized (getLockObject()) { + pushScreenCapturePolicy(userId); + } pushUserRestrictions(userId); // When system user is started (device boot), load cache for all users. // This is to mitigate the potential race between loading the cache and keyguard @@ -6919,7 +6921,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { notifyResetProtectionPolicyChanged(frpAgentUid); } mLockSettingsInternal.refreshStrongAuthTimeout(parentId); - updateScreenCaptureDisabled(parentId, getScreenCaptureDisabled(null, parentId, false)); Slogf.i(LOG_TAG, "Cleaning up device-wide policies done."); } @@ -7686,10 +7687,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (ap.disableScreenCapture != disabled) { ap.disableScreenCapture = disabled; saveSettingsLocked(caller.getUserId()); - final int affectedUserId = parent - ? getProfileParentId(caller.getUserId()) - : caller.getUserId(); - updateScreenCaptureDisabled(affectedUserId, disabled); + pushScreenCapturePolicy(caller.getUserId()); } } DevicePolicyEventLogger @@ -7699,6 +7697,38 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { .write(); } + // Push the screen capture policy for a given userId. If screen capture is disabled by the + // DO or COPE PO on the parent profile, then this takes precedence as screen capture will + // be disabled device-wide. + private void pushScreenCapturePolicy(int adminUserId) { + // Update screen capture device-wide if disabled by the DO or COPE PO on the parent profile. + ActiveAdmin admin = + getDeviceOwnerOrProfileOwnerOfOrganizationOwnedDeviceParentLocked( + UserHandle.USER_SYSTEM); + if (admin != null && admin.disableScreenCapture) { + setScreenCaptureDisabled(UserHandle.USER_ALL); + } else { + // Otherwise, update screen capture only for the calling user. + admin = getProfileOwnerAdminLocked(adminUserId); + if (admin != null && admin.disableScreenCapture) { + setScreenCaptureDisabled(adminUserId); + } else { + setScreenCaptureDisabled(UserHandle.USER_NULL); + } + } + } + + // Set the latest screen capture policy, overriding any existing ones. + // userHandle can be one of USER_ALL, USER_NULL or a concrete userId. + private void setScreenCaptureDisabled(int userHandle) { + int current = mPolicyCache.getScreenCaptureDisallowedUser(); + if (userHandle == current) { + return; + } + mPolicyCache.setScreenCaptureDisallowedUser(userHandle); + updateScreenCaptureDisabled(); + } + /** * Returns whether or not screen capture is disabled for a given admin, or disabled for any * active admin (if given admin is null). @@ -7708,7 +7738,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (!mHasFeature) { return false; } - final CallerIdentity caller = getCallerIdentity(who); Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)); @@ -7716,29 +7745,13 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Preconditions.checkCallAuthorization( isProfileOwnerOfOrganizationOwnedDevice(getCallerIdentity().getUserId())); } - - synchronized (getLockObject()) { - if (who != null) { - ActiveAdmin admin = getActiveAdminUncheckedLocked(who, userHandle, parent); - return (admin != null) && admin.disableScreenCapture; - } - - final int affectedUserId = parent ? getProfileParentId(userHandle) : userHandle; - List admins = getActiveAdminsForAffectedUserLocked(affectedUserId); - for (ActiveAdmin admin: admins) { - if (admin.disableScreenCapture) { - return true; - } - } - return false; - } + return !mPolicyCache.isScreenCaptureAllowed(userHandle); } - private void updateScreenCaptureDisabled(int userHandle, boolean disabled) { - mPolicyCache.setScreenCaptureAllowed(userHandle, !disabled); + private void updateScreenCaptureDisabled() { mHandler.post(() -> { try { - mInjector.getIWindowManager().refreshScreenCaptureDisabled(userHandle); + mInjector.getIWindowManager().refreshScreenCaptureDisabled(); } catch (RemoteException e) { Slogf.w(LOG_TAG, "Unable to notify WindowManager.", e); } @@ -8690,6 +8703,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } ActiveAdmin getDeviceOwnerOrProfileOwnerOfOrganizationOwnedDeviceLocked(int userId) { + ensureLocked(); ActiveAdmin admin = getDeviceOwnerAdminLocked(); if (admin == null) { admin = getProfileOwnerOfOrganizationOwnedDeviceLocked(userId); @@ -8697,6 +8711,16 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return admin; } + ActiveAdmin getDeviceOwnerOrProfileOwnerOfOrganizationOwnedDeviceParentLocked(int userId) { + ensureLocked(); + ActiveAdmin admin = getDeviceOwnerAdminLocked(); + if (admin != null) { + return admin; + } + admin = getProfileOwnerOfOrganizationOwnedDeviceLocked(userId); + return admin != null ? admin.getParentActiveAdmin() : null; + } + @Override public void clearDeviceOwner(String packageName) { Objects.requireNonNull(packageName, "packageName is null"); @@ -15151,6 +15175,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { saveSettingsLocked(userHandle); updateMaximumTimeToLockLocked(userHandle); policy.mRemovingAdmins.remove(adminReceiver); + pushScreenCapturePolicy(userHandle); Slogf.i(LOG_TAG, "Device admin " + adminReceiver + " removed from user " + userHandle); } diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index 45d101a4a4269..545361c934d2b 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -4992,8 +4992,8 @@ public class DevicePolicyManagerTest extends DpmTestBase { .thenReturn(12345 /* some UID in user 0 */); // Make personal apps look suspended dpms.getUserData(UserHandle.USER_SYSTEM).mAppsSuspended = true; - - clearInvocations(getServices().iwindowManager); + // Screen capture + dpm.setScreenCaptureDisabled(admin1, true); dpm.wipeData(0); verify(getServices().userManagerInternal).removeUserEvenWhenDisallowed(CALLER_USER_HANDLE); @@ -5004,6 +5004,8 @@ public class DevicePolicyManagerTest extends DpmTestBase { verify(getServices().userManager).setUserRestriction( UserManager.DISALLOW_ADD_USER, false, UserHandle.SYSTEM); + clearInvocations(getServices().iwindowManager); + // Some device-wide policies are getting cleaned-up after the user is removed. mContext.binder.callingUid = DpmMockContext.SYSTEM_UID; sendBroadcastWithUser(dpms, Intent.ACTION_USER_REMOVED, CALLER_USER_HANDLE); @@ -5020,9 +5022,10 @@ public class DevicePolicyManagerTest extends DpmTestBase { MockUtils.checkIntentAction( DevicePolicyManager.ACTION_RESET_PROTECTION_POLICY_CHANGED), MockUtils.checkUserHandle(UserHandle.USER_SYSTEM)); - // Refresh strong auth timeout and screen capture + // Refresh strong auth timeout verify(getServices().lockSettingsInternal).refreshStrongAuthTimeout(UserHandle.USER_SYSTEM); - verify(getServices().iwindowManager).refreshScreenCaptureDisabled(UserHandle.USER_SYSTEM); + // Refresh screen capture + verify(getServices().iwindowManager).refreshScreenCaptureDisabled(); // Unsuspend personal apps verify(getServices().packageManagerInternal) .unsuspendForSuspendingPackage(PLATFORM_PACKAGE_NAME, UserHandle.USER_SYSTEM);