Merge "enforce stricter rules when registering phoneAccounts" into rvc-dev am: 06920b605c (1c0c5632) · Commits · e / os / android_packages_services_Telecomm

src/com/android/server/telecom/PhoneAccountRegistrar.java

+169 −14

Original line number	Diff line number	Diff line
		@@ -29,6 +29,7 @@ import android.graphics.Bitmap;
		import android.graphics.BitmapFactory;
		import android.graphics.drawable.Icon;
		import android.net.Uri;
		import android.os.Binder;
		import android.os.Bundle;
		import android.os.AsyncTask;
		import android.os.PersistableBundle;
		@@ -132,9 +133,14 @@ public class PhoneAccountRegistrar {
		}

		public static final String FILE_NAME = "phone-account-registrar-state.xml";
		public static final String ICON_ERROR_MSG =
		"Icon cannot be written to memory. Try compressing or downsizing";
		@VisibleForTesting
		public static final int EXPECTED_STATE_VERSION = 9;
		public static final int MAX_PHONE_ACCOUNT_REGISTRATIONS = 10;
		public static final int MAX_PHONE_ACCOUNT_EXTRAS_KEY_PAIR_LIMIT = 100;
		public static final int MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT = 256;
		public static final int MAX_SCHEMES_PER_ACCOUNT = 10;

		/** Keep in sync with the same in SipSettings.java */
		private static final String SIP_SHARED_PREFERENCES = "SIP_PREFERENCES";
		@@ -751,6 +757,15 @@ public class PhoneAccountRegistrar {
		return getPhoneAccountHandles(0, null, packageName, false, userHandle);
		}


		/**
		* includes disabled, includes crossUserAccess
		*/
		public List<PhoneAccountHandle> getAllPhoneAccountHandlesForPackage(UserHandle userHandle,
		String packageName) {
		return getPhoneAccountHandles(0, null, packageName, true /* includeDisabled */, userHandle);
		}

		/**
		* Determines if a {@link PhoneAccountHandle} is for a self-managed {@link ConnectionService}.
		* @param handle The handle.
		@@ -770,8 +785,11 @@ public class PhoneAccountRegistrar {
		* Performs checks before calling addOrReplacePhoneAccount(PhoneAccount)
		*
		* @param account The {@code PhoneAccount} to add or replace.
		* @throws SecurityException if package does not have BIND_TELECOM_CONNECTION_SERVICE permission
		* @throws SecurityException if package does not have BIND_TELECOM_CONNECTION_SERVICE
		* permission
		* @throws IllegalArgumentException if MAX_PHONE_ACCOUNT_REGISTRATIONS are reached
		* @throws IllegalArgumentException if MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT is reached
		* @throws IllegalArgumentException if writing the Icon to memory will cause an Exception
		*/
		public void registerPhoneAccount(PhoneAccount account) {
		// Enforce the requirement that a connection service for a phone account has the correct
		@@ -783,21 +801,155 @@ public class PhoneAccountRegistrar {
		throw new SecurityException("PhoneAccount connection service requires "
		+ "BIND_TELECOM_CONNECTION_SERVICE permission.");
		}
		//Enforce an upper bound on the number of PhoneAccount's a package can register.
		// Most apps should only require 1-2.
		if (getPhoneAccountsForPackage(
		account.getAccountHandle().getComponentName().getPackageName(),
		account.getAccountHandle().getUserHandle()).size()
		enforceCharacterLimit(account);
		enforceIconSizeLimit(account);
		enforceMaxPhoneAccountLimit(account);
		addOrReplacePhoneAccount(account);
		}

		/**
		* Enforce an upper bound on the number of PhoneAccount's a package can register.
		* Most apps should only require 1-2. * Include disabled accounts.
		*
		* @param account to enforce check on
		* @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).size()
		>= MAX_PHONE_ACCOUNT_REGISTRATIONS) {
		Log.w(this, "Phone account %s reached max registration limit for package",
		account.getAccountHandle());
		EventLog.writeEvent(0x534e4554, "259064622", Binder.getCallingUid(),
		"enforceMaxPhoneAccountLimit");
		throw new IllegalArgumentException(
		"Error, cannot register phone account " + account.getAccountHandle()
		+ " because the limit, " + MAX_PHONE_ACCOUNT_REGISTRATIONS
		+ ", has been reached");
		}
		}
		/**
		* determine if there will be an issue writing the icon to memory
		*
		* @param account to enforce check on
		* @throws IllegalArgumentException if writing the Icon to memory will cause an Exception
		*/
		@VisibleForTesting
		public void enforceIconSizeLimit(PhoneAccount account) {
		if (account.getIcon() == null) {
		return;
		}
		String text = "";
		// convert the icon into a Base64 String
		try {
		text = XmlSerialization.writeIconToBase64String(account.getIcon());
		} catch (IOException e) {
		EventLog.writeEvent(0x534e4554, "259064622", Binder.getCallingUid(),
		"enforceIconSizeLimit");
		throw new IllegalArgumentException(ICON_ERROR_MSG);
		}
		}

		addOrReplacePhoneAccount(account);
		/**
		* All {@link PhoneAccount} and{@link PhoneAccountHandle} String and Char-Sequence fields
		* should be restricted to character limit of MAX_PHONE_ACCOUNT_CHAR_LIMIT to prevent exceptions
		* when writing large character streams to XML-Serializer.
		*
		* @param account to enforce character limit checks on
		* @throws IllegalArgumentException if MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT reached
		*/
		public void enforceCharacterLimit(PhoneAccount account) {
		if (account == null) {
		return;
		}
		PhoneAccountHandle handle = account.getAccountHandle();

		String[] fields =
		{"Package Name", "Class Name", "PhoneAccountHandle Id", "Label", "ShortDescription",
		"GroupId", "Address", "SubscriptionAddress"};
		CharSequence[] args = {handle.getComponentName().getPackageName(),
		handle.getComponentName().getClassName(), handle.getId(), account.getLabel(),
		account.getShortDescription(), account.getGroupId(),
		(account.getAddress() != null ? account.getAddress().toString() : ""),
		(account.getSubscriptionAddress() != null ?
		account.getSubscriptionAddress().toString() : "")};

		for (int i = 0; i < fields.length; i++) {
		if (args[i] != null && args[i].length() > MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT) {
		EventLog.writeEvent(0x534e4554, "259064622", Binder.getCallingUid(),
		"enforceCharacterLimit");
		throw new IllegalArgumentException("The PhoneAccount or PhoneAccountHandle"
		+ fields[i] + " field has an invalid character count. PhoneAccount and "
		+ "PhoneAccountHandle String and Char-Sequence fields are limited to "
		+ MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT + " characters.");
		}
		}

		// Enforce limits on the URI Schemes provided
		enforceLimitsOnSchemes(account);

		// Enforce limit on the PhoneAccount#mExtras
		Bundle extras = account.getExtras();
		if (extras != null) {
		if (extras.keySet().size() > MAX_PHONE_ACCOUNT_EXTRAS_KEY_PAIR_LIMIT) {
		EventLog.writeEvent(0x534e4554, "259064622", Binder.getCallingUid(),
		"enforceCharacterLimit");
		throw new IllegalArgumentException("The PhoneAccount#mExtras is limited to " +
		MAX_PHONE_ACCOUNT_EXTRAS_KEY_PAIR_LIMIT + " (key,value) pairs.");
		}

		for (String key : extras.keySet()) {
		Object value = extras.get(key);

		if ((key != null && key.length() > MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT) \|\|
		(value instanceof String &&
		((String) value).length() > MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT)) {
		EventLog.writeEvent(0x534e4554, "259064622", Binder.getCallingUid(),
		"enforceCharacterLimit");
		throw new IllegalArgumentException("The PhoneAccount#mExtras contains a String"
		+ " key or value that has an invalid character count. PhoneAccount and "
		+ "PhoneAccountHandle String and Char-Sequence fields are limited to "
		+ MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT + " characters.");
		}
		}
		}
		}

		/**
		* Enforce a character limit on all PA and PAH string or char-sequence fields.
		*
		* @param account to enforce check on
		* @throws IllegalArgumentException if MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT reached
		*/
		@VisibleForTesting
		public void enforceLimitsOnSchemes(@NonNull PhoneAccount account) {
		List<String> schemes = account.getSupportedUriSchemes();

		if (schemes == null) {
		return;
		}

		if (schemes.size() > MAX_SCHEMES_PER_ACCOUNT) {
		EventLog.writeEvent(0x534e4554, "259064622", Binder.getCallingUid(),
		"enforceLimitsOnSchemes");
		throw new IllegalArgumentException(
		"Error, cannot register phone account " + account.getAccountHandle()
		+ " because the URI scheme limit of "
		+ MAX_SCHEMES_PER_ACCOUNT + " has been reached");
		}

		for (String scheme : schemes) {
		if (scheme.length() > MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT) {
		EventLog.writeEvent(0x534e4554, "259064622", Binder.getCallingUid(),
		"enforceLimitsOnSchemes");
		throw new IllegalArgumentException(
		"Error, cannot register phone account " + account.getAccountHandle()
		+ " because the max scheme limit of "
		+ MAX_PHONE_ACCOUNT_FIELD_CHAR_LIMIT + " has been reached");
		}
		}
		}

		/**
		@@ -1521,17 +1673,20 @@ public class PhoneAccountRegistrar {
		protected void writeIconIfNonNull(String tagName, Icon value, XmlSerializer serializer)
		throws IOException {
		if (value != null) {
		ByteArrayOutputStream stream = new ByteArrayOutputStream();
		value.writeToStream(stream);
		byte[] iconByteArray = stream.toByteArray();
		String text = Base64.encodeToString(iconByteArray, 0, iconByteArray.length, 0);

		String text = writeIconToBase64String(value);
		serializer.startTag(null, tagName);
		serializer.text(text);
		serializer.endTag(null, tagName);
		}
		}

		public static String writeIconToBase64String(Icon icon) throws IOException {
		ByteArrayOutputStream stream = new ByteArrayOutputStream();
		icon.writeToStream(stream);
		byte[] iconByteArray = stream.toByteArray();
		return Base64.encodeToString(iconByteArray, 0, iconByteArray.length, 0);
		}

		protected void writeLong(String tagName, long value, XmlSerializer serializer)
		throws IOException {
		serializer.startTag(null, tagName);

src/com/android/server/telecom/TelecomServiceImpl.java

+3 −1

Original line number	Diff line number	Diff line
		@@ -66,7 +66,9 @@ import com.android.server.telecom.settings.BlockedNumbersActivity;
		import java.io.FileDescriptor;
		import java.io.PrintWriter;
		import java.util.Collections;
		import java.util.HashSet;
		import java.util.List;
		import java.util.Set;

		// TODO: Needed for move to system service: import com.android.internal.R;

		@@ -306,7 +308,7 @@ public class TelecomServiceImpl {
		try {
		Log.startSession("TSI.gPAFP");
		return new ParceledListSlice<>(mPhoneAccountRegistrar
		.getPhoneAccountsForPackage(packageName, callingUserHandle));
		.getAllPhoneAccountHandlesForPackage(callingUserHandle, packageName));
		} catch (Exception e) {
		Log.e(this, e, "getPhoneAccountsForPackage %s", packageName);
		throw e;

tests/src/com/android/server/telecom/tests/PhoneAccountRegistrarTest.java

+101 −0

Original line number	Diff line number	Diff line
		@@ -22,8 +22,18 @@ import static org.junit.Assert.assertNotNull;
		import static org.junit.Assert.assertNull;
		import static org.junit.Assert.assertTrue;
		import static org.junit.Assert.fail;
		import static org.mockito.ArgumentMatchers.any;
		import static org.mockito.ArgumentMatchers.anyObject;
		import static org.mockito.ArgumentMatchers.isA;
		import static org.mockito.Matchers.anyInt;
		import static org.mockito.Matchers.anyString;
		import static org.mockito.Mockito.clearInvocations;
		import static org.mockito.Mockito.doThrow;
		import static org.mockito.Mockito.mock;
		import static org.mockito.Mockito.never;
		import static org.mockito.Mockito.spy;
		import static org.mockito.Mockito.times;
		import static org.mockito.Mockito.verify;
		import static org.mockito.Mockito.when;

		import android.content.ComponentName;
		@@ -70,6 +80,8 @@ import java.io.BufferedOutputStream;
		import java.io.ByteArrayInputStream;
		import java.io.ByteArrayOutputStream;
		import java.io.File;
		import java.io.IOException;
		import java.io.OutputStream;
		import java.util.ArrayList;
		import java.util.Arrays;
		import java.util.List;
		@@ -82,6 +94,9 @@ public class PhoneAccountRegistrarTest extends TelecomTestCase {
		private static final int MAX_VERSION = Integer.MAX_VALUE;
		private static final String FILE_NAME = "phone-account-registrar-test-1223.xml";
		private static final String TEST_LABEL = "right";
		private static final String TEST_ID = "123";
		private final String PACKAGE_1 = "PACKAGE_1";
		private final String PACKAGE_2 = "PACKAGE_2";
		private PhoneAccountRegistrar mRegistrar;
		@Mock private TelecomManager mTelecomManager;
		@Mock private DefaultDialerCache mDefaultDialerCache;
		@@ -1006,6 +1021,92 @@ public class PhoneAccountRegistrarTest extends TelecomTestCase {
		assertFalse(PhoneAccountHandle.areFromSamePackage(null, d));
		}

		/**
		* Ensure an IllegalArgumentException is thrown when adding more than 10 schemes for a single
		* account
		*/
		@Test
		public void testLimitOnSchemeCount() {
		PhoneAccountHandle handle = makeQuickAccountHandle(TEST_ID);
		PhoneAccount.Builder builder = new PhoneAccount.Builder(handle, TEST_LABEL);
		for (int i = 0; i < PhoneAccountRegistrar.MAX_PHONE_ACCOUNT_REGISTRATIONS + 1; i++) {
		builder.addSupportedUriScheme(Integer.toString(i));
		}
		try {
		mRegistrar.enforceLimitsOnSchemes(builder.build());
		fail("should have hit exception in enforceLimitOnSchemes");
		} catch (IllegalArgumentException e) {
		// pass test
		}
		}

		/**
		* Ensure an IllegalArgumentException is thrown when adding more 256 chars for a single
		* account
		*/
		@Test
		public void testLimitOnSchemeLength() {
		PhoneAccountHandle handle = makeQuickAccountHandle(TEST_ID);
		PhoneAccount.Builder builder = new PhoneAccount.Builder(handle, TEST_LABEL);
		builder.addSupportedUriScheme(generateStringOfLen(257));
		try {
		mRegistrar.enforceLimitsOnSchemes(builder.build());
		fail("should have hit exception in enforceLimitOnSchemes");
		} catch (IllegalArgumentException e) {
		// pass test
		}
		}

		/**
		* Ensure an IllegalArgumentException is thrown when adding an address over the limit
		*/
		@Test
		public void testLimitOnAddress() {
		String text = generateStringOfLen(100);
		PhoneAccountHandle handle = makeQuickAccountHandle(TEST_ID);
		PhoneAccount.Builder builder = new PhoneAccount.Builder(handle,TEST_LABEL)
		.setAddress(Uri.fromParts(text, text, text));
		try {
		mRegistrar.enforceCharacterLimit(builder.build());
		fail("failed to throw IllegalArgumentException");
		} catch (IllegalArgumentException e) {
		// pass test
		}
		finally {
		mRegistrar.unregisterPhoneAccount(handle);
		}
		}

		/**
		* Ensure an IllegalArgumentException is thrown when an Icon that throws an IOException is given
		*/
		@Test
		public void testLimitOnIcon() throws Exception {
		Icon mockIcon = mock(Icon.class);
		// GIVEN
		PhoneAccount.Builder builder = new PhoneAccount.Builder(
		makeQuickAccountHandle(TEST_ID), TEST_LABEL).setIcon(mockIcon);
		try {
		// WHEN
		Mockito.doThrow(new IOException())
		.when(mockIcon).writeToStream(any(OutputStream.class));
		//THEN
		mRegistrar.enforceIconSizeLimit(builder.build());
		fail("failed to throw IllegalArgumentException");
		} catch (IllegalArgumentException e) {
		// pass test
		assertTrue(e.getMessage().contains(PhoneAccountRegistrar.ICON_ERROR_MSG));
		}
		}

		private String generateStringOfLen(int len){
		StringBuilder sb = new StringBuilder();
		for(int i=0; i < len; i++){
		sb.append("a");
		}
		return sb.toString();
		}

		private static ComponentName makeQuickConnectionServiceComponentName() {
		return new ComponentName(
		"com.android.server.telecom.tests",

tests/src/com/android/server/telecom/tests/TelecomServiceImplTest.java

+1 −1

Original line number	Diff line number	Diff line
		@@ -452,7 +452,7 @@ public class TelecomServiceImplTest extends TelecomTestCase {
		add(SIP_PA_HANDLE_17);
		}};
		when(mFakePhoneAccountRegistrar
		.getPhoneAccountsForPackage(anyString(), any(UserHandle.class)))
		.getAllPhoneAccountHandlesForPackage(any(UserHandle.class), anyString()))
		.thenReturn(phoneAccountHandleList);
		makeAccountsVisibleToAllUsers(TEL_PA_HANDLE_16, SIP_PA_HANDLE_17);
		assertEquals(phoneAccountHandleList,