diff options
| author | 2023-05-29 00:09:02 +0000 | |
|---|---|---|
| committer | 2023-05-31 23:04:38 +0000 | |
| commit | b98b6f55cdd6b52c07bdea70c08b937507bce1dc (patch) | |
| tree | 8d474e6caddcbc2fbe189df3daa61a43ac68e532 | |
| parent | 90ca501cdfb2563d80d404fdf092516d7605e1bf (diff) | |
Check content protection blocklist when fetching options
BYPASS_INCLUSIVE_LANGUAGE_REASON=All issues are in the existing code
which is already in production. New code has no issues.
Bug: 275732576
Test: Added new tests
Change-Id: I4f62a63aba34dcfcc1a76a0b4b58f4d1f5432f82
4 files changed, 299 insertions, 18 deletions
diff --git a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java index 51359add8fd1..9ff0746ebd19 100644 --- a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java +++ b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java @@ -88,11 +88,14 @@ import android.view.contentcapture.IContentCaptureOptionsCallback; import android.view.contentcapture.IDataShareWriteAdapter; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.infra.AbstractRemoteService; import com.android.internal.infra.GlobalWhitelistState; import com.android.internal.os.IResultReceiver; import com.android.internal.util.DumpUtils; import com.android.server.LocalServices; +import com.android.server.contentprotection.ContentProtectionBlocklistManager; +import com.android.server.contentprotection.ContentProtectionPackageManager; import com.android.server.infra.AbstractMasterSystemService; import com.android.server.infra.FrameworkResourcesServiceNameResolver; @@ -117,7 +120,7 @@ import java.util.concurrent.atomic.AtomicBoolean; * with other sources to provide contextual data in other areas of the system * such as Autofill. */ -public final class ContentCaptureManagerService extends +public class ContentCaptureManagerService extends AbstractMasterSystemService<ContentCaptureManagerService, ContentCapturePerUserService> { private static final String TAG = ContentCaptureManagerService.class.getSimpleName(); @@ -205,6 +208,8 @@ public final class ContentCaptureManagerService extends final GlobalContentCaptureOptions mGlobalContentCaptureOptions = new GlobalContentCaptureOptions(); + @Nullable private final ContentProtectionBlocklistManager mContentProtectionBlocklistManager; + public ContentCaptureManagerService(@NonNull Context context) { super(context, new FrameworkResourcesServiceNameResolver(context, com.android.internal.R.string.config_defaultContentCaptureService), @@ -242,6 +247,14 @@ public final class ContentCaptureManagerService extends mServiceNameResolver.getServiceName(userId), mServiceNameResolver.isTemporary(userId)); } + + if (getEnableContentProtectionReceiverLocked()) { + mContentProtectionBlocklistManager = createContentProtectionBlocklistManager(context); + mContentProtectionBlocklistManager.updateBlocklist( + mDevCfgContentProtectionAppsBlocklistSize); + } else { + mContentProtectionBlocklistManager = null; + } } @Override // from AbstractMasterSystemService @@ -397,7 +410,9 @@ public final class ContentCaptureManagerService extends } } - private void setFineTuneParamsFromDeviceConfig() { + /** @hide */ + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) + protected void setFineTuneParamsFromDeviceConfig() { synchronized (mLock) { mDevCfgMaxBufferSize = DeviceConfig.getInt( @@ -443,6 +458,8 @@ public final class ContentCaptureManagerService extends ContentCaptureManager .DEVICE_CONFIG_PROPERTY_CONTENT_PROTECTION_APPS_BLOCKLIST_SIZE, ContentCaptureManager.DEFAULT_CONTENT_PROTECTION_APPS_BLOCKLIST_SIZE); + // mContentProtectionBlocklistManager.updateBlocklist not called on purpose here to keep + // it immutable at this point mDevCfgContentProtectionBufferSize = DeviceConfig.getInt( DeviceConfig.NAMESPACE_CONTENT_CAPTURE, @@ -754,6 +771,25 @@ public final class ContentCaptureManagerService extends mGlobalContentCaptureOptions.dump(prefix2, pw); } + /** + * Used by the constructor in order to be able to override the value in the tests. + * + * @hide + */ + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) + @GuardedBy("mLock") + protected boolean getEnableContentProtectionReceiverLocked() { + return mDevCfgEnableContentProtectionReceiver; + } + + /** @hide */ + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) + @NonNull + protected ContentProtectionBlocklistManager createContentProtectionBlocklistManager( + @NonNull Context context) { + return new ContentProtectionBlocklistManager(new ContentProtectionPackageManager(context)); + } + final class ContentCaptureManagerServiceStub extends IContentCaptureManager.Stub { @Override @@ -1075,14 +1111,21 @@ public final class ContentCaptureManagerService extends @GuardedBy("mGlobalWhitelistStateLock") public ContentCaptureOptions getOptions(@UserIdInt int userId, @NonNull String packageName) { - boolean packageWhitelisted; + boolean isContentCaptureReceiverEnabled; + boolean isContentProtectionReceiverEnabled; ArraySet<ComponentName> whitelistedComponents = null; + synchronized (mGlobalWhitelistStateLock) { - packageWhitelisted = isWhitelisted(userId, packageName); - if (!packageWhitelisted) { - // Full package is not allowlisted: check individual components first + isContentCaptureReceiverEnabled = + isContentCaptureReceiverEnabled(userId, packageName); + isContentProtectionReceiverEnabled = + isContentProtectionReceiverEnabled(packageName); + + if (!isContentCaptureReceiverEnabled) { + // Full package is not allowlisted: check individual components next whitelistedComponents = getWhitelistedComponents(userId, packageName); - if (whitelistedComponents == null + if (!isContentProtectionReceiverEnabled + && whitelistedComponents == null && packageName.equals(mServicePackages.get(userId))) { // No components allowlisted either, but let it go because it's the // service's own package @@ -1101,7 +1144,9 @@ public final class ContentCaptureManagerService extends } } - if (!packageWhitelisted && whitelistedComponents == null) { + if (!isContentCaptureReceiverEnabled + && !isContentProtectionReceiverEnabled + && whitelistedComponents == null) { // No can do! if (verbose) { Slog.v(TAG, "getOptionsForPackage(" + packageName + "): not whitelisted"); @@ -1118,9 +1163,9 @@ public final class ContentCaptureManagerService extends mDevCfgTextChangeFlushingFrequencyMs, mDevCfgLogHistorySize, mDevCfgDisableFlushForViewTreeAppearing, - /* enableReceiver= */ true, + isContentCaptureReceiverEnabled || whitelistedComponents != null, new ContentCaptureOptions.ContentProtectionOptions( - mDevCfgEnableContentProtectionReceiver, + isContentProtectionReceiverEnabled, mDevCfgContentProtectionBufferSize), whitelistedComponents); if (verbose) Slog.v(TAG, "getOptionsForPackage(" + packageName + "): " + options); @@ -1141,6 +1186,35 @@ public final class ContentCaptureManagerService extends } } } + + @Override // from GlobalWhitelistState + public boolean isWhitelisted(@UserIdInt int userId, @NonNull String packageName) { + return isContentCaptureReceiverEnabled(userId, packageName) + || isContentProtectionReceiverEnabled(packageName); + } + + @Override // from GlobalWhitelistState + public boolean isWhitelisted(@UserIdInt int userId, @NonNull ComponentName componentName) { + return super.isWhitelisted(userId, componentName) + || isContentProtectionReceiverEnabled(componentName.getPackageName()); + } + + private boolean isContentCaptureReceiverEnabled( + @UserIdInt int userId, @NonNull String packageName) { + return super.isWhitelisted(userId, packageName); + } + + private boolean isContentProtectionReceiverEnabled(@NonNull String packageName) { + if (mContentProtectionBlocklistManager == null) { + return false; + } + synchronized (mLock) { + if (!mDevCfgEnableContentProtectionReceiver) { + return false; + } + } + return mContentProtectionBlocklistManager.isAllowed(packageName); + } } private static class DataShareCallbackDelegate extends IDataShareCallback.Stub { diff --git a/services/contentcapture/java/com/android/server/contentprotection/ContentProtectionBlocklistManager.java b/services/contentcapture/java/com/android/server/contentprotection/ContentProtectionBlocklistManager.java index 715cf9a8807e..a0fd28b3f279 100644 --- a/services/contentcapture/java/com/android/server/contentprotection/ContentProtectionBlocklistManager.java +++ b/services/contentcapture/java/com/android/server/contentprotection/ContentProtectionBlocklistManager.java @@ -35,7 +35,7 @@ import java.util.stream.Collectors; * * @hide */ -class ContentProtectionBlocklistManager { +public class ContentProtectionBlocklistManager { private static final String TAG = "ContentProtectionBlocklistManager"; @@ -46,7 +46,7 @@ class ContentProtectionBlocklistManager { @Nullable private Set<String> mPackageNameBlocklist; - protected ContentProtectionBlocklistManager( + public ContentProtectionBlocklistManager( @NonNull ContentProtectionPackageManager contentProtectionPackageManager) { mContentProtectionPackageManager = contentProtectionPackageManager; } diff --git a/services/contentcapture/java/com/android/server/contentprotection/ContentProtectionPackageManager.java b/services/contentcapture/java/com/android/server/contentprotection/ContentProtectionPackageManager.java index 1847e5d708a3..4ebac07ec3ea 100644 --- a/services/contentcapture/java/com/android/server/contentprotection/ContentProtectionPackageManager.java +++ b/services/contentcapture/java/com/android/server/contentprotection/ContentProtectionPackageManager.java @@ -43,7 +43,7 @@ public class ContentProtectionPackageManager { @NonNull private final PackageManager mPackageManager; - ContentProtectionPackageManager(@NonNull Context context) { + public ContentProtectionPackageManager(@NonNull Context context) { mPackageManager = context.getPackageManager(); } diff --git a/services/tests/servicestests/src/com/android/server/contentcapture/ContentCaptureManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/contentcapture/ContentCaptureManagerServiceTest.java index c872a11e7988..0e92d2248a7a 100644 --- a/services/tests/servicestests/src/com/android/server/contentcapture/ContentCaptureManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/contentcapture/ContentCaptureManagerServiceTest.java @@ -18,8 +18,16 @@ package com.android.server.contentcapture; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; +import android.annotation.NonNull; +import android.content.ComponentName; import android.content.ContentCaptureOptions; import android.content.Context; import android.content.pm.UserInfo; @@ -29,6 +37,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.SmallTest; import com.android.server.LocalServices; +import com.android.server.contentprotection.ContentProtectionBlocklistManager; import com.android.server.pm.UserManagerInternal; import com.google.common.collect.ImmutableList; @@ -56,12 +65,21 @@ public class ContentCaptureManagerServiceTest { private static final String PACKAGE_NAME = "com.test.package"; + private static final ComponentName COMPONENT_NAME = + new ComponentName(PACKAGE_NAME, "TestClass"); + private static final Context sContext = ApplicationProvider.getApplicationContext(); @Rule public final MockitoRule mMockitoRule = MockitoJUnit.rule(); @Mock private UserManagerInternal mMockUserManagerInternal; + @Mock private ContentProtectionBlocklistManager mMockContentProtectionBlocklistManager; + + private boolean mDevCfgEnableContentProtectionReceiver; + + private int mContentProtectionBlocklistManagersCreated; + private ContentCaptureManagerService mContentCaptureManagerService; @Before @@ -69,20 +87,77 @@ public class ContentCaptureManagerServiceTest { when(mMockUserManagerInternal.getUserInfos()).thenReturn(new UserInfo[0]); LocalServices.removeServiceForTest(UserManagerInternal.class); LocalServices.addService(UserManagerInternal.class, mMockUserManagerInternal); - mContentCaptureManagerService = new ContentCaptureManagerService(sContext); + mContentCaptureManagerService = new TestContentCaptureManagerService(); } @Test - public void getOptions_notAllowlisted() { + public void constructor_default_doesNotCreateContentProtectionBlocklistManager() { + assertThat(mContentProtectionBlocklistManagersCreated).isEqualTo(0); + verifyZeroInteractions(mMockContentProtectionBlocklistManager); + } + + @Test + public void constructor_flagDisabled_doesNotContentProtectionBlocklistManager() { + assertThat(mContentProtectionBlocklistManagersCreated).isEqualTo(0); + verifyZeroInteractions(mMockContentProtectionBlocklistManager); + } + + @Test + public void constructor_flagEnabled_createsContentProtectionBlocklistManager() { + mDevCfgEnableContentProtectionReceiver = true; + + mContentCaptureManagerService = new TestContentCaptureManagerService(); + + assertThat(mContentProtectionBlocklistManagersCreated).isEqualTo(1); + verify(mMockContentProtectionBlocklistManager).updateBlocklist(anyInt()); + } + + @Test + public void setFineTuneParamsFromDeviceConfig_doesNotUpdateContentProtectionBlocklist() { + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); + mContentCaptureManagerService.mDevCfgContentProtectionAppsBlocklistSize += 100; + verify(mMockContentProtectionBlocklistManager).updateBlocklist(anyInt()); + + mContentCaptureManagerService.setFineTuneParamsFromDeviceConfig(); + + verifyNoMoreInteractions(mMockContentProtectionBlocklistManager); + } + + @Test + public void getOptions_contentCaptureDisabled_contentProtectionDisabled() { + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); + ContentCaptureOptions actual = mContentCaptureManagerService.mGlobalContentCaptureOptions.getOptions( USER_ID, PACKAGE_NAME); assertThat(actual).isNull(); + verify(mMockContentProtectionBlocklistManager).isAllowed(PACKAGE_NAME); } @Test - public void getOptions_allowlisted_contentCaptureReceiver() { + public void getOptions_contentCaptureDisabled_contentProtectionEnabled() { + when(mMockContentProtectionBlocklistManager.isAllowed(PACKAGE_NAME)).thenReturn(true); + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); + + ContentCaptureOptions actual = + mContentCaptureManagerService.mGlobalContentCaptureOptions.getOptions( + USER_ID, PACKAGE_NAME); + + assertThat(actual).isNotNull(); + assertThat(actual.enableReceiver).isFalse(); + assertThat(actual.contentProtectionOptions).isNotNull(); + assertThat(actual.contentProtectionOptions.enableReceiver).isTrue(); + assertThat(actual.whitelistedComponents).isNull(); + } + + @Test + public void getOptions_contentCaptureEnabled_contentProtectionDisabled() { + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); mContentCaptureManagerService.mGlobalContentCaptureOptions.setWhitelist( USER_ID, ImmutableList.of(PACKAGE_NAME), /* components= */ null); @@ -92,13 +167,17 @@ public class ContentCaptureManagerServiceTest { assertThat(actual).isNotNull(); assertThat(actual.enableReceiver).isTrue(); + assertThat(actual.contentProtectionOptions).isNotNull(); assertThat(actual.contentProtectionOptions.enableReceiver).isFalse(); assertThat(actual.whitelistedComponents).isNull(); + verify(mMockContentProtectionBlocklistManager).isAllowed(PACKAGE_NAME); } @Test - public void getOptions_allowlisted_bothReceivers() { - mContentCaptureManagerService.mDevCfgEnableContentProtectionReceiver = true; + public void getOptions_contentCaptureEnabled_contentProtectionEnabled() { + when(mMockContentProtectionBlocklistManager.isAllowed(PACKAGE_NAME)).thenReturn(true); + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); mContentCaptureManagerService.mGlobalContentCaptureOptions.setWhitelist( USER_ID, ImmutableList.of(PACKAGE_NAME), /* components= */ null); @@ -108,7 +187,135 @@ public class ContentCaptureManagerServiceTest { assertThat(actual).isNotNull(); assertThat(actual.enableReceiver).isTrue(); + assertThat(actual.contentProtectionOptions).isNotNull(); assertThat(actual.contentProtectionOptions.enableReceiver).isTrue(); assertThat(actual.whitelistedComponents).isNull(); } + + @Test + public void isWhitelisted_packageName_contentCaptureDisabled_contentProtectionDisabled() { + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); + + boolean actual = + mContentCaptureManagerService.mGlobalContentCaptureOptions.isWhitelisted( + USER_ID, PACKAGE_NAME); + + assertThat(actual).isFalse(); + verify(mMockContentProtectionBlocklistManager).isAllowed(PACKAGE_NAME); + } + + @Test + public void isWhitelisted_packageName_contentCaptureDisabled_contentProtectionEnabled() { + when(mMockContentProtectionBlocklistManager.isAllowed(PACKAGE_NAME)).thenReturn(true); + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); + + boolean actual = + mContentCaptureManagerService.mGlobalContentCaptureOptions.isWhitelisted( + USER_ID, PACKAGE_NAME); + + assertThat(actual).isTrue(); + } + + @Test + public void isWhitelisted_packageName_contentCaptureEnabled_contentProtectionNotChecked() { + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); + mContentCaptureManagerService.mGlobalContentCaptureOptions.setWhitelist( + USER_ID, ImmutableList.of(PACKAGE_NAME), /* components= */ null); + + boolean actual = + mContentCaptureManagerService.mGlobalContentCaptureOptions.isWhitelisted( + USER_ID, PACKAGE_NAME); + + assertThat(actual).isTrue(); + verify(mMockContentProtectionBlocklistManager, never()).isAllowed(anyString()); + } + + @Test + public void isWhitelisted_componentName_contentCaptureDisabled_contentProtectionDisabled() { + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); + + boolean actual = + mContentCaptureManagerService.mGlobalContentCaptureOptions.isWhitelisted( + USER_ID, COMPONENT_NAME); + + assertThat(actual).isFalse(); + verify(mMockContentProtectionBlocklistManager).isAllowed(PACKAGE_NAME); + } + + @Test + public void isWhitelisted_componentName_contentCaptureDisabled_contentProtectionEnabled() { + when(mMockContentProtectionBlocklistManager.isAllowed(PACKAGE_NAME)).thenReturn(true); + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); + + boolean actual = + mContentCaptureManagerService.mGlobalContentCaptureOptions.isWhitelisted( + USER_ID, COMPONENT_NAME); + + assertThat(actual).isTrue(); + } + + @Test + public void isWhitelisted_componentName_contentCaptureEnabled_contentProtectionNotChecked() { + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); + mContentCaptureManagerService.mGlobalContentCaptureOptions.setWhitelist( + USER_ID, /* packageNames= */ null, ImmutableList.of(COMPONENT_NAME)); + + boolean actual = + mContentCaptureManagerService.mGlobalContentCaptureOptions.isWhitelisted( + USER_ID, COMPONENT_NAME); + + assertThat(actual).isTrue(); + verify(mMockContentProtectionBlocklistManager, never()).isAllowed(anyString()); + } + + @Test + public void isContentProtectionReceiverEnabled_withoutBlocklistManager() { + boolean actual = + mContentCaptureManagerService.mGlobalContentCaptureOptions.isWhitelisted( + USER_ID, PACKAGE_NAME); + + assertThat(actual).isFalse(); + verify(mMockContentProtectionBlocklistManager, never()).isAllowed(anyString()); + } + + @Test + public void isContentProtectionReceiverEnabled_disabledWithFlag() { + mDevCfgEnableContentProtectionReceiver = true; + mContentCaptureManagerService = new TestContentCaptureManagerService(); + mContentCaptureManagerService.mDevCfgEnableContentProtectionReceiver = false; + + boolean actual = + mContentCaptureManagerService.mGlobalContentCaptureOptions.isWhitelisted( + USER_ID, PACKAGE_NAME); + + assertThat(actual).isFalse(); + verify(mMockContentProtectionBlocklistManager, never()).isAllowed(anyString()); + } + + private class TestContentCaptureManagerService extends ContentCaptureManagerService { + + TestContentCaptureManagerService() { + super(sContext); + this.mDevCfgEnableContentProtectionReceiver = + ContentCaptureManagerServiceTest.this.mDevCfgEnableContentProtectionReceiver; + } + + @Override + protected boolean getEnableContentProtectionReceiverLocked() { + return ContentCaptureManagerServiceTest.this.mDevCfgEnableContentProtectionReceiver; + } + + @Override + protected ContentProtectionBlocklistManager createContentProtectionBlocklistManager( + @NonNull Context context) { + mContentProtectionBlocklistManagersCreated++; + return mMockContentProtectionBlocklistManager; + } + } } |