Merge "enforce stricter rules when registering phoneAccounts" (6bc45802) · Commits · e / os / android_packages_services_Telecomm

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

+136 −27

Original line number	Diff line number	Diff line
		@@ -32,6 +32,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;
		@@ -59,6 +60,7 @@ import android.util.Xml;
		import com.android.internal.annotations.VisibleForTesting;
		import com.android.internal.util.IndentingPrintWriter;
		import com.android.internal.util.XmlUtils;
		import com.android.modules.utils.ModifiedUtf8;

		import org.xmlpull.v1.XmlPullParser;
		import org.xmlpull.v1.XmlPullParserException;
		@@ -151,11 +153,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_EXTAS_KEY_PAIR_LIMIT = 100;
		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";
		@@ -811,6 +816,16 @@ public class PhoneAccountRegistrar {
		return getPhoneAccountHandles(0, null, packageName, false, userHandle, false);
		}


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

		/**
		* Retrieves a list of all {@link PhoneAccount#CAPABILITY_SELF_MANAGED} phone accounts
		* registered by a specified package.
		@@ -849,8 +864,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
		@@ -863,28 +881,69 @@ 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.
		int numberRegisteredPhoneAccountsForPackage = getPhoneAccountsForPackage(
		account.getAccountHandle().getComponentName().getPackageName(),
		account.getAccountHandle().getUserHandle()).size();
		Log.i(this, "registerPhoneAccount: The number of phone accounts currently"
		+ " registered by this package is %s. The maximum allowable number is %s.",
		numberRegisteredPhoneAccountsForPackage, MAX_PHONE_ACCOUNT_REGISTRATIONS);
		if (numberRegisteredPhoneAccountsForPackage >= MAX_PHONE_ACCOUNT_REGISTRATIONS) {
		Log.w(this, "Phone account %s reached max registration limit for package",
		account.getAccountHandle());
		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, false / crossUserAccess */).size()
		>= MAX_PHONE_ACCOUNT_REGISTRATIONS) {
		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");
		}
		// Enforce a character limit on all PA and PAH string or char-sequence fields.
		enforceCharacterLimit(account);
		}

		Log.i(this, "registerPhoneAccount: Adding or replacing PhoneAccount=%s",
		account);
		addOrReplacePhoneAccount(account);
		/**
		* 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);
		}
		// enforce the max bytes check in com.android.modules.utils.FastDataOutput#writeUTF(string)
		try {
		final int len = (int) ModifiedUtf8.countBytes(text, false);
		if (len > 65_535 /* MAX_UNSIGNED_SHORT */) {
		EventLog.writeEvent(0x534e4554, "259064622", Binder.getCallingUid(),
		"enforceIconSizeLimit");
		throw new IllegalArgumentException(ICON_ERROR_MSG);
		}
		} catch (IOException e) {
		EventLog.writeEvent(0x534e4554, "259064622", Binder.getCallingUid(),
		"enforceIconSizeLimit");
		throw new IllegalArgumentException(ICON_ERROR_MSG);
		}
		}

		/**
		@@ -893,6 +952,7 @@ public class PhoneAccountRegistrar {
		* 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) {
		@@ -902,13 +962,16 @@ public class PhoneAccountRegistrar {

		String[] fields =
		{"Package Name", "Class Name", "PhoneAccountHandle Id", "Label", "ShortDescription",
		"GroupId"};
		"GroupId", "Address"};
		CharSequence[] args = {handle.getComponentName().getPackageName(),
		handle.getComponentName().getClassName(), handle.getId(), account.getLabel(),
		account.getShortDescription(), account.getGroupId()};
		account.getShortDescription(), account.getGroupId(),
		(account.getAddress() != null ? account.getAddress().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 "
		@@ -916,11 +979,17 @@ public class PhoneAccountRegistrar {
		}
		}

		// 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_EXTAS_KEY_PAIR_LIMIT) {
		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_EXTAS_KEY_PAIR_LIMIT + " (key,value) pairs.");
		MAX_PHONE_ACCOUNT_EXTRAS_KEY_PAIR_LIMIT + " (key,value) pairs.");
		}

		for (String key : extras.keySet()) {
		@@ -929,6 +998,8 @@ public class PhoneAccountRegistrar {
		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 "
		@@ -938,6 +1009,41 @@ public class PhoneAccountRegistrar {
		}
		}

		/**
		* 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");
		}
		}
		}

		/**
		* Adds a {@code PhoneAccount}, replacing an existing one if found.
		*
		@@ -1858,17 +1964,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

+1 −7

Original line number	Diff line number	Diff line
		@@ -60,13 +60,9 @@ import android.provider.Settings;
		import android.telecom.CallAttributes;

		import android.telecom.CallException;
		import android.telecom.CallAttributes;
		import android.telecom.CallException;
		import android.telecom.CallStreamingService;
		import android.telecom.Log;
		import android.telecom.PhoneAccount;
		import android.telecom.PhoneAccountHandle;
		import android.telecom.StreamingCall;
		import android.telecom.TelecomAnalytics;
		import android.telecom.TelecomManager;
		import android.telecom.VideoProfile;
		@@ -92,10 +88,8 @@ import com.android.server.telecom.voip.VoipCallTransactionResult;

		import java.io.FileDescriptor;
		import java.io.PrintWriter;
		import java.util.Collections;
		import java.util.HashSet;
		import java.util.List;
		import java.util.Locale;
		import java.util.Set;

		// TODO: Needed for move to system service: import com.android.internal.R;
		@@ -469,7 +463,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

+88 −2

Original line number	Diff line number	Diff line
		@@ -22,11 +22,17 @@ 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.anyBoolean;
		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;
		@@ -83,6 +89,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.Collection;
		@@ -1560,7 +1568,7 @@ public class PhoneAccountRegistrarTest extends TelecomTestCase {
		public void testInvalidExtraElementsExceedsLimitAndThrowsException() {
		// GIVEN
		int invalidBundleExtrasLimit =
		PhoneAccountRegistrar.MAX_PHONE_ACCOUNT_EXTAS_KEY_PAIR_LIMIT + 1;
		PhoneAccountRegistrar.MAX_PHONE_ACCOUNT_EXTRAS_KEY_PAIR_LIMIT + 1;
		Bundle extras = new Bundle();
		for (int i = 0; i < invalidBundleExtrasLimit; i++) {
		extras.putString(UUID.randomUUID().toString(), "value");
		@@ -1581,6 +1589,84 @@ public class PhoneAccountRegistrarTest extends TelecomTestCase {
		}
		}

		/**
		* 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(INVALID_STR);
		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 = "a".repeat(100);
		PhoneAccountHandle handle = makeQuickAccountHandle(TEST_ID);
		PhoneAccount.Builder builder = makeBuilderWithBindCapabilities(handle)
		.setAddress(Uri.fromParts(text, text, text));
		try {
		mRegistrar.registerPhoneAccount(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 = makeBuilderWithBindCapabilities(
		makeQuickAccountHandle(TEST_ID)).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 static PhoneAccount.Builder makeBuilderWithBindCapabilities(PhoneAccountHandle handle) {
		return new PhoneAccount.Builder(handle, TEST_LABEL)
		.setCapabilities(PhoneAccount.CAPABILITY_SUPPORTS_TRANSACTIONAL_OPERATIONS);

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

+1 −1

Original line number	Diff line number	Diff line
		@@ -532,7 +532,7 @@ public class TelecomServiceImplTest extends TelecomTestCase {
		List<PhoneAccountHandle> phoneAccountHandleList = List.of(
		TEL_PA_HANDLE_16, 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,