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

Commit 52970b2d authored by Jack Yu's avatar Jack Yu
Browse files

Fixed ANR in PinStorage 

Moved secret key initialization code into worker thread.

Test: atest FrameworkTelephonyTests
Flag: EXEMPT bug fix
Bug: 331549683
Change-Id: I79b2b78a2004d2c75a4a2e8838b9b365e2ae4e3c
parent 57bfd806

src/java/com/android/internal/telephony/uicc/PinStorage.java

+17 −6
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ import static com.android.internal.telephony.uicc.IccCardStatus.PinState.PINSTAT 
import static com.android.internal.telephony.uicc.IccCardStatus.PinState.PINSTATE_ENABLED_VERIFIED;

import static com.android.internal.telephony.util.TelephonyUtils.FORCE_VERBOSE_STATE_LOGGING;



import android.annotation.NonNull;

import android.annotation.Nullable;

import android.app.KeyguardManager;

import android.content.BroadcastReceiver;
@@ -47,6 +48,7 @@ import android.content.IntentFilter; 
import android.content.SharedPreferences;

import android.os.AsyncResult;

import android.os.Handler;

import android.os.Looper;

import android.os.Message;

import android.os.PersistableBundle;

import android.os.WorkSource;
@@ -66,10 +68,12 @@ import com.android.internal.telephony.Phone; 
import com.android.internal.telephony.PhoneConstants;

import com.android.internal.telephony.PhoneFactory;

import com.android.internal.telephony.TelephonyStatsLog;

import com.android.internal.telephony.flags.FeatureFlags;

import com.android.internal.telephony.nano.StoredPinProto.EncryptedPin;

import com.android.internal.telephony.nano.StoredPinProto.StoredPin;

import com.android.internal.telephony.nano.StoredPinProto.StoredPin.PinStatus;

import com.android.internal.telephony.uicc.IccCardStatus.PinState;

import com.android.internal.telephony.util.WorkerThread;

import com.android.internal.util.ArrayUtils;

import com.android.telephony.Rlog;
@@ -180,7 +184,8 @@ public class PinStorage extends Handler { 
        }

    };



    public PinStorage(Context context) {

    public PinStorage(Context context, @NonNull Looper looper, @NonNull FeatureFlags featureFlags) {

        super(looper);

        mContext = context;

        mBootCount = getBootCount();

        mKeyStore = initializeKeyStore();
@@ -211,10 +216,16 @@ public class PinStorage extends Handler { 
        String alias = (!mIsDeviceSecure || mIsDeviceLocked)

                ? KEYSTORE_ALIAS_LONG_TERM_ALWAYS : KEYSTORE_ALIAS_LONG_TERM_USER_AUTH;

        // This is the main thread, so accessing keystore in a separate thread to prevent ANR.

        Executors.newSingleThreadExecutor().execute(() -> mLongTermSecretKey = initializeSecretKey(

        if (featureFlags.threadShred()) {

            WorkerThread.getExecutor().execute(() -> mLongTermSecretKey = initializeSecretKey(

                    alias, /*createIfAbsent=*/ true));

        } else {

            Executors.newSingleThreadExecutor()

                    .execute(() -> mLongTermSecretKey = initializeSecretKey(

                            alias, /*createIfAbsent=*/ true));

        }



        // If the device is not securee or is unlocked, we can start logic. Otherwise we need to

        // If the device is not secured or is unlocked, we can start logic. Otherwise we need to

        // wait for the device to be unlocked and store any temporary PIN in RAM.

        if (!mIsDeviceSecure || !mIsDeviceLocked) {

            mRamStorage = null;
@@ -673,7 +684,7 @@ public class PinStorage extends Handler { 
        } else {

            // Load both the stored PIN in available state (with long-term key) and in other states

            // (with short-term key). At most one of them should be present at any given time and

            // we treat the case wheere both are present as an error.

            // we treat the case where both are present as an error.

            StoredPin availableStoredPin = loadPinInformationFromDisk(

                    slotId, SHARED_PREFS_AVAILABLE_PIN_BASE_KEY, mLongTermSecretKey);

            StoredPin rebootStoredPin = loadPinInformationFromDisk(

src/java/com/android/internal/telephony/uicc/UiccController.java

+8 −1
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ import android.content.SharedPreferences; 
import android.content.pm.PackageManager;

import android.os.AsyncResult;

import android.os.Handler;

import android.os.Looper;

import android.os.Message;

import android.os.RegistrantList;

import android.os.UserHandle;
@@ -72,6 +73,7 @@ import com.android.internal.telephony.subscription.SubscriptionManagerService; 
import com.android.internal.telephony.uicc.euicc.EuiccCard;

import com.android.internal.telephony.util.ArrayUtils;

import com.android.internal.telephony.util.TelephonyUtils;

import com.android.internal.telephony.util.WorkerThread;

import com.android.telephony.Rlog;



import java.io.FileDescriptor;
@@ -305,7 +307,12 @@ public class UiccController extends Handler { 
        PhoneConfigurationManager.registerForMultiSimConfigChange(

                this, EVENT_MULTI_SIM_CONFIG_CHANGED, null);



        mPinStorage = new PinStorage(mContext);

        if (mFeatureFlags.threadShred()) {

            mPinStorage = new PinStorage(mContext, WorkerThread.getHandler().getLooper(),

                    mFeatureFlags);

        } else {

            mPinStorage = new PinStorage(mContext, Looper.myLooper(), mFeatureFlags);

        }

        if (!TelephonyUtils.IS_USER) {

            mUseRemovableEsimAsDefault = PreferenceManager.getDefaultSharedPreferences(mContext)

                    .getBoolean(REMOVABLE_ESIM_AS_DEFAULT, false);

tests/telephonytests/src/com/android/internal/telephony/TelephonyTest.java

+1 −0
Original line number Diff line number Diff line
@@ -565,6 +565,7 @@ public abstract class TelephonyTest { 
        lenient().doReturn(true).when(mFeatureFlags).dataServiceCheck();

        lenient().doReturn(true).when(mFeatureFlags).phoneTypeCleanup();

        lenient().doReturn(true).when(mFeatureFlags).cleanupCdma();

        lenient().doReturn(true).when(mFeatureFlags).threadShred();



        WorkerThread.reset();

        TelephonyManager.disableServiceHandleCaching();

tests/telephonytests/src/com/android/internal/telephony/uicc/PinStorageTest.java

+30 −42
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import static org.mockito.Mockito.verify; 
import static org.mockito.Mockito.when;



import android.content.Intent;

import android.os.Looper;

import android.os.PersistableBundle;

import android.os.WorkSource;

import android.preference.PreferenceManager;
@@ -38,7 +39,6 @@ import android.testing.AndroidTestingRunner; 
import android.testing.TestableLooper;



import androidx.test.InstrumentationRegistry;

import androidx.test.filters.SmallTest;



import com.android.internal.R;

import com.android.internal.telephony.PhoneConstants;
@@ -69,7 +69,7 @@ public class PinStorageTest extends TelephonyTest { 
    // mocks

    private CarrierConfigManager.CarrierConfigChangeListener mCarrierConfigChangeListener;



    private void simulateReboot() {

    private void simulateReboot() throws Exception {

        mSimulatedRebootsCount++;

        Settings.Global.putInt(mContext.getContentResolver(),

                Settings.Global.BOOT_COUNT, mBootCount + mSimulatedRebootsCount);
@@ -77,11 +77,12 @@ public class PinStorageTest extends TelephonyTest { 
        createPinStorageAndCaptureListener();

    }



    private void createPinStorageAndCaptureListener() {

    private void createPinStorageAndCaptureListener() throws Exception {

        // Capture listener to emulate the carrier config change notification used later

        ArgumentCaptor<CarrierConfigManager.CarrierConfigChangeListener> listenerArgumentCaptor =

                ArgumentCaptor.forClass(CarrierConfigManager.CarrierConfigChangeListener.class);

        mPinStorage = new PinStorage(mContext);

        mPinStorage = new PinStorage(mContext, Looper.myLooper(), mFeatureFlags);

        processAllMessages();

        mPinStorage.mShortTermSecretKeyDurationMinutes = 0;

        verify(mCarrierConfigManager, atLeastOnce()).registerCarrierConfigChangeListener(any(),

                listenerArgumentCaptor.capture());
@@ -132,7 +133,6 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_withoutReboot_pinCannotBeRetrieved() {

        mPinStorage.storePin("1234", 0);
@@ -140,8 +140,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_normalReboot_pinCannotBeRetrieved() {

    public void storePin_normalReboot_pinCannotBeRetrieved() throws Exception {

        mPinStorage.storePin("1234", 0);



        simulateReboot();
@@ -150,23 +149,23 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_crash_pinCannotBeRetrieved() {

    public void storePin_crash_pinCannotBeRetrieved() throws Exception {

        mPinStorage.storePin("1234", 0);



        // Simulate crash

        mPinStorage = new PinStorage(mContext);

        mPinStorage = new PinStorage(mContext, Looper.myLooper(), mFeatureFlags);

        processAllMessages();

        mPinStorage.mShortTermSecretKeyDurationMinutes = 0;



        assertThat(mPinStorage.getPin(0, ICCID_1)).isEqualTo("");

    }



    @Test

    @SmallTest

    public void storePin_unattendedReboot_pinCanBeRetrievedOnce() {

    public void storePin_unattendedReboot_pinCanBeRetrievedOnce() throws Exception {

        mPinStorage.storePin("1234", 0);



        int result = mPinStorage.prepareUnattendedReboot(sWorkSource);

        processAllMessages();

        assertThat(result).isEqualTo(TelephonyManager.PREPARE_UNATTENDED_REBOOT_SUCCESS);



        simulateReboot();
@@ -177,8 +176,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_unattendedReboot_deviceIsLocked() {

    public void storePin_unattendedReboot_deviceIsLocked() throws Exception {

        // Simulate the device is still locked

        when(mKeyguardManager.isDeviceSecure()).thenReturn(true);

        when(mKeyguardManager.isDeviceLocked()).thenReturn(true);
@@ -196,8 +194,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_unattendedReboot_pinIsRemovedAfterDelay() {

    public void storePin_unattendedReboot_pinIsRemovedAfterDelay() throws Exception {

        mPinStorage.storePin("1234", 0);



        int result = mPinStorage.prepareUnattendedReboot(sWorkSource);
@@ -221,11 +218,11 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_unattendedRebootNotDone_pinCannotBeRetrieved() {

    public void storePin_unattendedRebootNotDone_pinCannotBeRetrieved() throws Exception {

        mPinStorage.storePin("1234", 0);



        int result = mPinStorage.prepareUnattendedReboot(sWorkSource);

        processAllMessages();

        assertThat(result).isEqualTo(TelephonyManager.PREPARE_UNATTENDED_REBOOT_SUCCESS);



        // Move time forward by 60 seconds before simulating reboot
@@ -237,8 +234,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_unattendedReboot_iccidChange() {

    public void storePin_unattendedReboot_iccidChange() throws Exception {

        mPinStorage.storePin("1234", 0);



        int result = mPinStorage.prepareUnattendedReboot(sWorkSource);
@@ -258,8 +254,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void clearPin_pinCannotBeRetrieved() {

    public void clearPin_pinCannotBeRetrieved() throws Exception {

        mPinStorage.storePin("1234", 0);

        mPinStorage.clearPin(0);
@@ -272,12 +267,12 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_pinChanged_pinIsUpdated() {

    public void storePin_pinChanged_pinIsUpdated() throws Exception {

        mPinStorage.storePin("1234", 0);

        mPinStorage.storePin("5678", 0);



        int result = mPinStorage.prepareUnattendedReboot(sWorkSource);

        processAllMessages();

        assertThat(result).isEqualTo(TelephonyManager.PREPARE_UNATTENDED_REBOOT_SUCCESS);



        simulateReboot();
@@ -286,8 +281,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_pinTooShort_pinIsNotStored() {

    public void storePin_pinTooShort_pinIsNotStored() throws Exception {

        mPinStorage.storePin("12", 0);



        int result = mPinStorage.prepareUnattendedReboot(sWorkSource);
@@ -299,8 +293,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_pinTooLong_pinIsNotStored() {

    public void storePin_pinTooLong_pinIsNotStored() throws Exception {

        mPinStorage.storePin("123456789", 0);



        int result = mPinStorage.prepareUnattendedReboot(sWorkSource);
@@ -312,8 +305,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_invalidIccid_pinIsNotStored() {

    public void storePin_invalidIccid_pinIsNotStored() throws Exception {

        doReturn(ICCID_INVALID).when(mPhone).getFullIccSerialNumber();



        mPinStorage.storePin("1234", 0);
@@ -325,8 +317,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_disabledInResources_pinIsNotStored() {

    public void storePin_disabledInResources_pinIsNotStored() throws Exception {

        mContextFixture.putBooleanResource(

                R.bool.config_allow_pin_storage_for_unattended_reboot, false);
@@ -341,8 +332,8 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_disabledInResources_containsSimWithPinEnabledAndVerified() {

    public void storePin_disabledInResources_containsSimWithPinEnabledAndVerified()

            throws Exception {

        mContextFixture.putBooleanResource(

                R.bool.config_allow_pin_storage_for_unattended_reboot, false);
@@ -360,8 +351,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_disabledInCarrierConfig_pinIsNotStored() {

    public void storePin_disabledInCarrierConfig_pinIsNotStored() throws Exception {

        PersistableBundle carrierConfigs = new PersistableBundle();

        carrierConfigs.putBoolean(

                CarrierConfigManager.KEY_STORE_SIM_PIN_FOR_UNATTENDED_REBOOT_BOOL, false);
@@ -378,8 +368,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_changeToDisabledInCarrierConfig_pinIsRemoved() {

    public void storePin_changeToDisabledInCarrierConfig_pinIsRemoved() throws Exception {

        mPinStorage.storePin("1234", 0);



        // Simulate change in the carrier configuration
@@ -401,8 +390,7 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_simIsRemoved_pinIsRemoved() {

    public void storePin_simIsRemoved_pinIsRemoved() throws Exception {

        mPinStorage.storePin("1234", 0);



        // SIM is removed
@@ -421,11 +409,11 @@ public class PinStorageTest extends TelephonyTest { 
    }



    @Test

    @SmallTest

    public void storePin_simReadyAfterUnattendedReboot_pinIsRemoved() {

    public void storePin_simReadyAfterUnattendedReboot_pinIsRemoved() throws Exception {

        mPinStorage.storePin("1234", 0);



        int result = mPinStorage.prepareUnattendedReboot(sWorkSource);

        processAllMessages();

        assertThat(result).isEqualTo(TelephonyManager.PREPARE_UNATTENDED_REBOOT_SUCCESS);



        simulateReboot();