enforcePhoneAccounts should unregister unresolvable accts
Accounts that cannot be resolved anymore should be unregistered to avoid
keeping the unusable account in memory.
Flag: com.android.server.telecom.flags.unregister_unresolvable_accounts
Fixes: 281061708
Test: e2e tests + unit tests + manual:
(1) flash apk that registers accts on diff CS's
(2) register MAX accts and disable CS
(3) app should repeat step 2 mutliple times
observe/expect: accts unregistered after disabling CS and
registering another acct
Change-Id: I57ddbef96cb56cd5e2349825123fe099ce0df106
diff --git a/flags/Android.bp b/flags/Android.bp
index d60a5f5..45acacf 100644
--- a/flags/Android.bp
+++ b/flags/Android.bp
@@ -42,5 +42,6 @@
"telecom_remote_connection_service.aconfig",
"telecom_profile_user_flags.aconfig",
"telecom_bluetoothdevicemanager_flags.aconfig",
+ "telecom_non_critical_security_flags.aconfig",
],
}
diff --git a/flags/telecom_non_critical_security_flags.aconfig b/flags/telecom_non_critical_security_flags.aconfig
new file mode 100644
index 0000000..37929a8
--- /dev/null
+++ b/flags/telecom_non_critical_security_flags.aconfig
@@ -0,0 +1,10 @@
+package: "com.android.server.telecom.flags"
+container: "system"
+
+# OWNER=tjstuart TARGET=24Q4
+flag {
+ name: "unregister_unresolvable_accounts"
+ namespace: "telecom"
+ description: "When set, Telecom will unregister accounts if the service is not resolvable"
+ bug: "281061708"
+}
\ No newline at end of file
diff --git a/src/com/android/server/telecom/PhoneAccountRegistrar.java b/src/com/android/server/telecom/PhoneAccountRegistrar.java
index 9256387..f0423c3 100644
--- a/src/com/android/server/telecom/PhoneAccountRegistrar.java
+++ b/src/com/android/server/telecom/PhoneAccountRegistrar.java
@@ -981,6 +981,9 @@
}
enforceCharacterLimit(account);
enforceIconSizeLimit(account);
+ if (mTelecomFeatureFlags.unregisterUnresolvableAccounts()) {
+ enforcePhoneAccountTargetService(account);
+ }
enforceMaxPhoneAccountLimit(account);
if (mTelephonyFeatureFlags.simultaneousCallingIndications()) {
enforceSimultaneousCallingRestrictionLimit(account);
@@ -989,6 +992,25 @@
}
/**
+ * This method ensures that {@link PhoneAccount}s that have the {@link
+ * PhoneAccount#CAPABILITY_SUPPORTS_TRANSACTIONAL_OPERATIONS} capability are not
+ * backed by a {@link ConnectionService}
+ *
+ * @param account enforce the check on
+ */
+ private void enforcePhoneAccountTargetService(PhoneAccount account) {
+ if (phoneAccountRequiresBindPermission(account.getAccountHandle()) &&
+ hasTransactionalCallCapabilities(account)) {
+ throw new IllegalArgumentException(
+ "Error, the PhoneAccount you are registering has"
+ + " CAPABILITY_SUPPORTS_TRANSACTIONAL_OPERATIONS and the"
+ + " PhoneAccountHandle's ComponentName#ClassName points to a"
+ + " ConnectionService class. Either remove the capability or use a"
+ + " different ClassName in the PhoneAccountHandle.");
+ }
+ }
+
+ /**
* Enforce an upper bound on the number of PhoneAccount's a package can register.
* Most apps should only require 1-2. * Include disabled accounts.
*
@@ -996,13 +1018,17 @@
* @throws IllegalArgumentException if MAX_PHONE_ACCOUNT_REGISTRATIONS are reached
*/
private void enforceMaxPhoneAccountLimit(@NonNull PhoneAccount account) {
- final PhoneAccountHandle accountHandle = account.getAccountHandle();
- final UserHandle user = accountHandle.getUserHandle();
- final ComponentName componentName = accountHandle.getComponentName();
-
- if (getPhoneAccountHandles(0, null, componentName.getPackageName(),
- true /* includeDisabled */, user, false /* crossUserAccess */).size()
- >= MAX_PHONE_ACCOUNT_REGISTRATIONS) {
+ int numOfAcctsRegisteredForPackage = mTelecomFeatureFlags.unregisterUnresolvableAccounts()
+ ? cleanupAndGetVerifiedAccounts(account).size()
+ : getPhoneAccountHandles(
+ 0/* capabilities */,
+ null /* uriScheme */,
+ account.getAccountHandle().getComponentName().getPackageName(),
+ true /* includeDisabled */,
+ account.getAccountHandle().getUserHandle(),
+ false /* crossUserAccess */).size();
+ // enforce the max phone account limit for the application registering accounts
+ if (numOfAcctsRegisteredForPackage >= MAX_PHONE_ACCOUNT_REGISTRATIONS) {
EventLog.writeEvent(0x534e4554, "259064622", Binder.getCallingUid(),
"enforceMaxPhoneAccountLimit");
throw new IllegalArgumentException(
@@ -1012,6 +1038,64 @@
}
}
+ @VisibleForTesting
+ public List<PhoneAccount> getRegisteredAccountsForPackageName(String packageName,
+ UserHandle userHandle) {
+ if (packageName == null) {
+ return new ArrayList<>();
+ }
+ List<PhoneAccount> accounts = new ArrayList<>(mState.accounts.size());
+ for (PhoneAccount m : mState.accounts) {
+ PhoneAccountHandle handle = m.getAccountHandle();
+ if (!packageName.equals(handle.getComponentName().getPackageName())) {
+ // Not the right package name; skip this one.
+ continue;
+ }
+ // Do not count accounts registered under different users on the device. Otherwise, an
+ // application can only have MAX_PHONE_ACCOUNT_REGISTRATIONS across all users. If the
+ // DUT has multiple users, they should each get to register 10 accounts. Also, 3rd
+ // party applications cannot create new UserHandles without highly privileged
+ // permissions.
+ if (!isVisibleForUser(m, userHandle, false)) {
+ // Account is not visible for the current user; skip this one.
+ continue;
+ }
+ accounts.add(m);
+ }
+ return accounts;
+ }
+
+ /**
+ * Unregister {@link ConnectionService} accounts that no longer have a resolvable Service. This
+ * means the Service has been disabled or died. Skip the verification for transactional
+ * accounts.
+ *
+ * @param newAccount being registered
+ * @return all the verified accounts. These accounts are now guaranteed to be backed by a
+ * {@link ConnectionService} or do not need one (transactional accounts).
+ */
+ @VisibleForTesting
+ public List<PhoneAccount> cleanupAndGetVerifiedAccounts(PhoneAccount newAccount) {
+ ArrayList<PhoneAccount> verifiedAccounts = new ArrayList<>();
+ List<PhoneAccount> unverifiedAccounts = getRegisteredAccountsForPackageName(
+ newAccount.getAccountHandle().getComponentName().getPackageName(),
+ newAccount.getAccountHandle().getUserHandle());
+ for (PhoneAccount account : unverifiedAccounts) {
+ PhoneAccountHandle handle = account.getAccountHandle();
+ if (/* skip for transactional accounts since they don't require a ConnectionService */
+ !hasTransactionalCallCapabilities(account) &&
+ /* check if the {@link ConnectionService} has been disabled or can longer be
+ found */ resolveComponent(handle).isEmpty()) {
+ Log.i(this, " cAGVA: Cannot resolve the ConnectionService for"
+ + " handle=[%s]; unregistering account", handle);
+ unregisterPhoneAccount(handle);
+ } else {
+ verifiedAccounts.add(account);
+ }
+ }
+ return verifiedAccounts;
+ }
+
/**
* determine if there will be an issue writing the icon to memory
*
diff --git a/tests/src/com/android/server/telecom/tests/PhoneAccountRegistrarTest.java b/tests/src/com/android/server/telecom/tests/PhoneAccountRegistrarTest.java
index 9d87aaf..45b4ed1 100644
--- a/tests/src/com/android/server/telecom/tests/PhoneAccountRegistrarTest.java
+++ b/tests/src/com/android/server/telecom/tests/PhoneAccountRegistrarTest.java
@@ -141,6 +141,7 @@
mComponentContextFixture.getTestDouble().getApplicationContext(), mLock, FILE_NAME,
mDefaultDialerCache, mAppLabelProxy, mTelephonyFeatureFlags, mFeatureFlags);
when(mFeatureFlags.onlyUpdateTelephonyOnValidSubIds()).thenReturn(false);
+ when(mFeatureFlags.unregisterUnresolvableAccounts()).thenReturn(true);
when(mTelephonyFeatureFlags.workProfileApiSplit()).thenReturn(false);
}
@@ -467,6 +468,60 @@
PhoneAccount.SCHEME_TEL));
}
+ /**
+ * Verify when a {@link android.telecom.ConnectionService} is disabled or cannot be resolved,
+ * all phone accounts are unregistered when calling
+ * {@link PhoneAccountRegistrar#cleanupAndGetVerifiedAccounts(PhoneAccount)}.
+ */
+ @Test
+ public void testCannotResolveServiceUnregistersAccounts() throws Exception {
+ ComponentName componentName = makeQuickConnectionServiceComponentName();
+ PhoneAccount account = makeQuickAccountBuilder("0", 0, USER_HANDLE_10)
+ .setCapabilities(PhoneAccount.CAPABILITY_CONNECTION_MANAGER
+ | PhoneAccount.CAPABILITY_CALL_PROVIDER).build();
+ // add the ConnectionService and register a single phone account for it
+ mComponentContextFixture.addConnectionService(componentName,
+ Mockito.mock(IConnectionService.class));
+ registerAndEnableAccount(account);
+ // verify the start state
+ assertEquals(1,
+ mRegistrar.getRegisteredAccountsForPackageName(componentName.getPackageName(),
+ USER_HANDLE_10).size());
+ // remove the ConnectionService so that the account cannot be resolved anymore
+ mComponentContextFixture.removeConnectionService(componentName,
+ Mockito.mock(IConnectionService.class));
+ // verify the account is unregistered when fetching the phone accounts for the package
+ assertEquals(1,
+ mRegistrar.getRegisteredAccountsForPackageName(componentName.getPackageName(),
+ USER_HANDLE_10).size());
+ assertEquals(0,
+ mRegistrar.cleanupAndGetVerifiedAccounts(account).size());
+ assertEquals(0,
+ mRegistrar.getRegisteredAccountsForPackageName(componentName.getPackageName(),
+ USER_HANDLE_10).size());
+ }
+
+ /**
+ * Verify that if a client adds both the {@link
+ * PhoneAccount#CAPABILITY_SUPPORTS_TRANSACTIONAL_OPERATIONS} capability AND is backed by a
+ * {@link android.telecom.ConnectionService}, a {@link IllegalArgumentException} is thrown.
+ */
+ @Test
+ public void testConnectionServiceAndTransactionalAccount() throws Exception {
+ PhoneAccount account = makeQuickAccountBuilder("0", 0, USER_HANDLE_10)
+ .setCapabilities(PhoneAccount.CAPABILITY_SELF_MANAGED
+ | PhoneAccount.CAPABILITY_SUPPORTS_TRANSACTIONAL_OPERATIONS).build();
+ mComponentContextFixture.addConnectionService(
+ makeQuickConnectionServiceComponentName(),
+ Mockito.mock(IConnectionService.class));
+ try {
+ registerAndEnableAccount(account);
+ fail("failed to throw IllegalArgumentException");
+ } catch (IllegalArgumentException e) {
+ // test passed, ignore Exception.
+ }
+ }
+
@MediumTest
@Test
public void testSimCallManager() throws Exception {