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

Commit ffd3c208 authored by Dmitry Dementyev's avatar Dmitry Dementyev
Browse files

Update recovery key protection params for secondary profile. 

Having extra protection for platform key key doesn't improve security
much. Anyone who is able to use key with new param will be able to
1) Use application keys directly without need to access encrypted
backups
2) Read data encrypted by application keys.

Test: atest com.android.server.locksettings.recoverablekeystore
Bug: 111561213
Change-Id: I9017bb4331298dc336c321a1491c9a25556c0051
parent ba3fdfdd

services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java

+2 −4
Original line number Diff line number Diff line
@@ -168,8 +168,7 @@ public class KeySyncTask implements Runnable { 
        if (mCredentialType == LockPatternUtils.CREDENTIAL_TYPE_NONE) {

            // Application keys for the user will not be available for sync.

            Log.w(TAG, "Credentials are not set for user " + mUserId);

            if (generation < PlatformKeyManager.MIN_GENERATION_ID_FOR_UNLOCKED_DEVICE_REQUIRED

                    || mUserId != UserHandle.USER_SYSTEM) {

            if (generation < PlatformKeyManager.MIN_GENERATION_ID_FOR_UNLOCKED_DEVICE_REQUIRED) {

                // Only invalidate keys with legacy protection param.

                mPlatformKeyManager.invalidatePlatformKey(mUserId, generation);

            }
@@ -178,8 +177,7 @@ public class KeySyncTask implements Runnable { 
        if (isCustomLockScreen()) {

            Log.w(TAG, "Unsupported credential type " + mCredentialType + " for user " + mUserId);

            // Keys will be synced when user starts using non custom screen lock.

            if (generation < PlatformKeyManager.MIN_GENERATION_ID_FOR_UNLOCKED_DEVICE_REQUIRED

                    || mUserId != UserHandle.USER_SYSTEM) {

            if (generation < PlatformKeyManager.MIN_GENERATION_ID_FOR_UNLOCKED_DEVICE_REQUIRED) {

                mRecoverableKeyStoreDb.invalidateKeysForUserIdOnCustomScreenLock(mUserId);

            }

            return;

services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java

+2 −16
Original line number Diff line number Diff line
@@ -67,7 +67,7 @@ import javax.crypto.spec.GCMParameterSpec; 
 * @hide

 */

public class PlatformKeyManager {

    static final int MIN_GENERATION_ID_FOR_UNLOCKED_DEVICE_REQUIRED = 1000000;

    static final int MIN_GENERATION_ID_FOR_UNLOCKED_DEVICE_REQUIRED = 1001000;



    private static final String TAG = "PlatformKeyManager";

    private static final String KEY_ALGORITHM = "AES";
@@ -434,21 +434,7 @@ public class PlatformKeyManager { 
        if (userId ==  UserHandle.USER_SYSTEM) {

            decryptionKeyProtection.setUnlockedDeviceRequired(true);

        } else {

            // With setUnlockedDeviceRequired, KeyStore thinks that device is locked .

            decryptionKeyProtection.setUserAuthenticationRequired(true);

            decryptionKeyProtection.setUserAuthenticationValidityDurationSeconds(

                            USER_AUTHENTICATION_VALIDITY_DURATION_SECONDS);

            // Bind decryption key to secondary profile lock screen secret.

            long secureUserId = getGateKeeperService().getSecureUserId(userId);

            // TODO(b/124095438): Propagate this failure instead of silently failing.

            if (secureUserId == GateKeeper.INVALID_SECURE_USER_ID) {

                Log.e(TAG, "No SID available for user " + userId);

                return;

            }

            decryptionKeyProtection

                    .setBoundToSpecificSecureUserId(secureUserId)

                    // Ignore caller uid which always belongs to the primary profile.

                    .setCriticalToDeviceEncryption(true);

            // Don't set protection params to prevent losing key.

        }

        // Store decryption key first since it is more likely to fail.

        mKeyStore.setEntry(

services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/PlatformKeyManagerTest.java

+13 −24
Original line number Diff line number Diff line
@@ -24,11 +24,9 @@ import static org.mockito.ArgumentMatchers.any; 
import static org.mockito.ArgumentMatchers.anyString;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.doThrow;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;

import static org.testng.Assert.expectThrows;



import android.app.KeyguardManager;

import android.content.Context;
@@ -67,7 +65,7 @@ import javax.crypto.KeyGenerator; 
public class PlatformKeyManagerTest {



    private static final String DATABASE_FILE_NAME = "recoverablekeystore.db";

    private static final int MIN_GENERATION_ID = 1000000;

    private static final int MIN_GENERATION_ID = 1001000;

    private static final int PRIMARY_USER_ID_FIXTURE = 0;

    private static final int USER_ID_FIXTURE = 42;

    private static final long USER_SID = 4200L;
@@ -76,15 +74,15 @@ public class PlatformKeyManagerTest { 
    private static final String TESTING_KEYSTORE_KEY_ALIAS = "testing-key-store-key-alias";



    private static final String ENCRYPTION_KEY_ALIAS_1 =

             "com.android.server.locksettings.recoverablekeystore/platform/42/1000000/encrypt";

             "com.android.server.locksettings.recoverablekeystore/platform/42/1001000/encrypt";

    private static final String DECRYPTION_KEY_ALIAS_1 =

             "com.android.server.locksettings.recoverablekeystore/platform/42/1000000/decrypt";

             "com.android.server.locksettings.recoverablekeystore/platform/42/1001000/decrypt";

    private static final String DECRYPTION_KEY_FOR_ALIAS_PRIMARY_USER_1 =

             "com.android.server.locksettings.recoverablekeystore/platform/0/1000000/decrypt";

             "com.android.server.locksettings.recoverablekeystore/platform/0/1001000/decrypt";

    private static final String ENCRYPTION_KEY_ALIAS_2 =

             "com.android.server.locksettings.recoverablekeystore/platform/42/1000001/encrypt";

             "com.android.server.locksettings.recoverablekeystore/platform/42/1001001/encrypt";

    private static final String DECRYPTION_KEY_ALIAS_2 =

             "com.android.server.locksettings.recoverablekeystore/platform/42/1000001/decrypt";

             "com.android.server.locksettings.recoverablekeystore/platform/42/1001001/decrypt";



    @Mock private Context mContext;

    @Mock private KeyStoreProxy mKeyStoreProxy;
@@ -221,42 +219,33 @@ public class PlatformKeyManagerTest { 
    }



    @Test

    public void init_secondaryUserUser_createsDecryptKeyWithAuthenticationRequired()

    public void init_secondaryUserUser_createsDecryptKeyWithoutAuthenticationRequired()

            throws Exception {

        mPlatformKeyManager.init(USER_ID_FIXTURE);



        assertTrue(getDecryptKeyProtection().isUserAuthenticationRequired());

        assertFalse(getDecryptKeyProtection().isUserAuthenticationRequired());

    }



    @Test

    public void init_createsDecryptKeyBoundToTheUsersAuthentication() throws Exception {

        mPlatformKeyManager.init(USER_ID_FIXTURE);



        assertEquals(

                USER_SID,

                getDecryptKeyProtection().getBoundToSpecificSecureUserId());

    }



    @Test

    public void init_doesNotCreateDecryptKeyIfNoSid() throws Exception {

    public void init_createsDecryptKeyIfNoSid() throws Exception {

        when(mGateKeeperService.getSecureUserId(USER_ID_FIXTURE))

                .thenReturn(GateKeeper.INVALID_SECURE_USER_ID);



        mPlatformKeyManager.init(USER_ID_FIXTURE);



        verify(mKeyStoreProxy, never()).setEntry(

        verify(mKeyStoreProxy).setEntry(

                eq(DECRYPTION_KEY_ALIAS_1),

                any(),

                any());

    }



    @Test

    public void init_doesNotCreateDecryptKeyOnGateKeeperException() throws Exception {

    public void init_createsDecryptKeyOnGateKeeperException() throws Exception {

        when(mGateKeeperService.getSecureUserId(USER_ID_FIXTURE)).thenThrow(new RemoteException());



        expectThrows(RemoteException.class, () -> mPlatformKeyManager.init(USER_ID_FIXTURE));

        mPlatformKeyManager.init(USER_ID_FIXTURE);



        verify(mKeyStoreProxy, never()).setEntry(

        verify(mKeyStoreProxy).setEntry(

                eq(DECRYPTION_KEY_ALIAS_1),

                any(),

                any());