Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7ba6643f authored by Thomas Stuart's avatar Thomas Stuart
Browse files

enforce stricter rules when registering phoneAccounts 

- include disable accounts when looking up accounts for a package to
  check if the limit is reached (10)
- put a new limit of 10 supported schemes
- put a new limit of 256 characters per scheme
- put a new limit of 256 characters per address
- ensure the Icon can write to memory w/o an exception

bug: 259064622
Test: cts + unit
Change-Id: I5eb2a127a44d5ec725d0ba39cb0ef478b12013de
parent c75c5d1a

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,