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

Commit 26f7222e authored by Dmitry Dementyev's avatar Dmitry Dementyev
Browse files

Throw InsecureUserException when LSKF is not set in recoverablekeystore. 

Without fix RecoveryController.generateKey and importKey throw generic ServiceSpecificException.

Bug: 283534188
Test: atest com.android.server.locksettings.recoverablekeystore
Change-Id: I8604d3c771e37ca3322d3301037b7443d0a3928b
parent 3a317d93

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

+32 −17
Original line number Original line Diff line number Diff line
@@ -166,6 +166,7 @@ public class PlatformKeyManager { 
     * @param userId The ID of the user to whose lock screen the platform key must be bound.

     * @param userId The ID of the user to whose lock screen the platform key must be bound.

     * @throws NoSuchAlgorithmException if AES is unavailable - should never happen.

     * @throws NoSuchAlgorithmException if AES is unavailable - should never happen.

     * @throws KeyStoreException if there is an error in AndroidKeyStore.

     * @throws KeyStoreException if there is an error in AndroidKeyStore.

     * @throws InsecureUserException if the user does not have a lock screen set.

     * @throws IOException if there was an issue with local database update.

     * @throws IOException if there was an issue with local database update.

     * @throws RemoteException if there was an issue communicating with {@link IGateKeeperService}.

     * @throws RemoteException if there was an issue communicating with {@link IGateKeeperService}.

     *

     *
@@ -174,7 +175,7 @@ public class PlatformKeyManager { 
    @VisibleForTesting

    @VisibleForTesting

    void regenerate(int userId)

    void regenerate(int userId)

            throws NoSuchAlgorithmException, KeyStoreException, IOException,

            throws NoSuchAlgorithmException, KeyStoreException, IOException,

                    RemoteException {

                    RemoteException, InsecureUserException {

        int generationId = getGenerationId(userId);

        int generationId = getGenerationId(userId);

        int nextId;

        int nextId;

        if (generationId == -1) {

        if (generationId == -1) {
@@ -195,13 +196,14 @@ public class PlatformKeyManager { 
     * @throws UnrecoverableKeyException if the key could not be recovered.

     * @throws UnrecoverableKeyException if the key could not be recovered.

     * @throws NoSuchAlgorithmException if AES is unavailable - should never occur.

     * @throws NoSuchAlgorithmException if AES is unavailable - should never occur.

     * @throws IOException if there was an issue with local database update.

     * @throws IOException if there was an issue with local database update.

     * @throws InsecureUserException if the user does not have a lock screen set.

     * @throws RemoteException if there was an issue communicating with {@link IGateKeeperService}.

     * @throws RemoteException if there was an issue communicating with {@link IGateKeeperService}.

     *

     *

     * @hide

     * @hide

     */

     */

    public PlatformEncryptionKey getEncryptKey(int userId)

    public PlatformEncryptionKey getEncryptKey(int userId)

            throws KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException,

            throws KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException,

                    IOException, RemoteException {

                    IOException, RemoteException, InsecureUserException {

        init(userId);

        init(userId);

        try {

        try {

            // Try to see if the decryption key is still accessible before using the encryption key.

            // Try to see if the decryption key is still accessible before using the encryption key.
@@ -254,7 +256,7 @@ public class PlatformKeyManager { 
     */

     */

    public PlatformDecryptionKey getDecryptKey(int userId)

    public PlatformDecryptionKey getDecryptKey(int userId)

            throws KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException,

            throws KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException,

                    IOException, RemoteException {

                    IOException, InsecureUserException, RemoteException {

        init(userId);

        init(userId);

        try {

        try {

            PlatformDecryptionKey decryptionKey = getDecryptKeyInternal(userId);

            PlatformDecryptionKey decryptionKey = getDecryptKeyInternal(userId);
@@ -328,7 +330,7 @@ public class PlatformKeyManager { 
     */

     */

    void init(int userId)

    void init(int userId)

            throws KeyStoreException, NoSuchAlgorithmException, IOException,

            throws KeyStoreException, NoSuchAlgorithmException, IOException,

                    RemoteException {

                    RemoteException, InsecureUserException {

        int generationId = getGenerationId(userId);

        int generationId = getGenerationId(userId);

        if (isKeyLoaded(userId, generationId)) {

        if (isKeyLoaded(userId, generationId)) {

            Log.i(TAG, String.format(

            Log.i(TAG, String.format(
@@ -414,7 +416,8 @@ public class PlatformKeyManager { 
     * @throws RemoteException if there was an issue communicating with {@link IGateKeeperService}.

     * @throws RemoteException if there was an issue communicating with {@link IGateKeeperService}.

     */

     */

    private void generateAndLoadKey(int userId, int generationId)

    private void generateAndLoadKey(int userId, int generationId)

            throws NoSuchAlgorithmException, KeyStoreException, IOException, RemoteException {

            throws NoSuchAlgorithmException, KeyStoreException, IOException, RemoteException,

                InsecureUserException {

        String encryptAlias = getEncryptAlias(userId, generationId);

        String encryptAlias = getEncryptAlias(userId, generationId);

        String decryptAlias = getDecryptAlias(userId, generationId);

        String decryptAlias = getDecryptAlias(userId, generationId);

        // SecretKey implementation doesn't provide reliable way to destroy the secret

        // SecretKey implementation doesn't provide reliable way to destroy the secret
@@ -427,11 +430,13 @@ public class PlatformKeyManager { 
                    .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE);

                    .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE);

        // Skip UserAuthenticationRequired for main user

        // Skip UserAuthenticationRequired for main user

        if (userId ==  UserHandle.USER_SYSTEM) {

        if (userId ==  UserHandle.USER_SYSTEM) {

            // attempt to store key will fail if screenlock is not set.

            decryptionKeyProtection.setUnlockedDeviceRequired(true);

            decryptionKeyProtection.setUnlockedDeviceRequired(true);

        } else {

        } else {

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

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

        }

        }

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

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

        try {

            mKeyStore.setEntry(

            mKeyStore.setEntry(

                    decryptAlias,

                    decryptAlias,

                    new KeyStore.SecretKeyEntry(secretKey),

                    new KeyStore.SecretKeyEntry(secretKey),
@@ -443,7 +448,13 @@ public class PlatformKeyManager { 
                        .setBlockModes(KeyProperties.BLOCK_MODE_GCM)

                        .setBlockModes(KeyProperties.BLOCK_MODE_GCM)

                        .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE)

                        .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE)

                        .build());

                        .build());



        } catch (KeyStoreException e) {

            if (!isDeviceSecure(userId)) {

                throw new InsecureUserException("Screenlock is not set");

            } else {

                throw e;

            }

        }

        setGenerationId(userId, generationId);

        setGenerationId(userId, generationId);

    }

    }
@@ -477,4 +488,8 @@ public class PlatformKeyManager { 
        return keyStore;

        return keyStore;

    }

    }





    private boolean isDeviceSecure(int userId) {

        return mContext.getSystemService(KeyguardManager.class).isDeviceSecure(userId);

    }



}
 
}

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

+5 −0
Original line number Original line Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.server.locksettings.recoverablekeystore; 
import static android.security.keystore.recovery.RecoveryController.ERROR_BAD_CERTIFICATE_FORMAT;

import static android.security.keystore.recovery.RecoveryController.ERROR_BAD_CERTIFICATE_FORMAT;

import static android.security.keystore.recovery.RecoveryController.ERROR_DECRYPTION_FAILED;

import static android.security.keystore.recovery.RecoveryController.ERROR_DECRYPTION_FAILED;

import static android.security.keystore.recovery.RecoveryController.ERROR_DOWNGRADE_CERTIFICATE;

import static android.security.keystore.recovery.RecoveryController.ERROR_DOWNGRADE_CERTIFICATE;

import static android.security.keystore.recovery.RecoveryController.ERROR_INSECURE_USER;

import static android.security.keystore.recovery.RecoveryController.ERROR_INVALID_CERTIFICATE;

import static android.security.keystore.recovery.RecoveryController.ERROR_INVALID_CERTIFICATE;

import static android.security.keystore.recovery.RecoveryController.ERROR_INVALID_KEY_FORMAT;

import static android.security.keystore.recovery.RecoveryController.ERROR_INVALID_KEY_FORMAT;

import static android.security.keystore.recovery.RecoveryController.ERROR_NO_SNAPSHOT_PENDING;

import static android.security.keystore.recovery.RecoveryController.ERROR_NO_SNAPSHOT_PENDING;
@@ -750,6 +751,8 @@ public class RecoverableKeyStoreManager { 
            throw new RuntimeException(e);

            throw new RuntimeException(e);

        } catch (KeyStoreException | UnrecoverableKeyException | IOException e) {

        } catch (KeyStoreException | UnrecoverableKeyException | IOException e) {

            throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR, e.getMessage());

            throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR, e.getMessage());

        } catch (InsecureUserException e) {

            throw new ServiceSpecificException(ERROR_INSECURE_USER, e.getMessage());

        }

        }





        try {

        try {
@@ -817,6 +820,8 @@ public class RecoverableKeyStoreManager { 
            throw new RuntimeException(e);

            throw new RuntimeException(e);

        } catch (KeyStoreException | UnrecoverableKeyException | IOException e) {

        } catch (KeyStoreException | UnrecoverableKeyException | IOException e) {

            throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR, e.getMessage());

            throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR, e.getMessage());

        } catch (InsecureUserException e) {

            throw new ServiceSpecificException(ERROR_INSECURE_USER, e.getMessage());

        }

        }





        try {

        try {

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

+14 −0
Original line number Original line Diff line number Diff line
@@ -27,6 +27,7 @@ import static org.mockito.Mockito.doThrow; 
import static org.mockito.Mockito.times;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;

import static org.mockito.Mockito.when;

import static org.testng.Assert.assertThrows;





import android.app.KeyguardManager;

import android.app.KeyguardManager;

import android.content.Context;

import android.content.Context;
@@ -54,6 +55,7 @@ import org.mockito.MockitoAnnotations; 




import java.io.File;

import java.io.File;

import java.security.KeyStore;

import java.security.KeyStore;

import java.security.KeyStoreException;

import java.security.UnrecoverableKeyException;

import java.security.UnrecoverableKeyException;

import java.util.List;

import java.util.List;
@@ -392,6 +394,18 @@ public class PlatformKeyManagerTest { 
                any());

                any());

    }

    }





    @Test

    public void getEncryptKey_noScreenlock() throws Exception {

        when(mKeyguardManager.isDeviceSecure(USER_ID_FIXTURE)).thenReturn(false);

        doThrow(new KeyStoreException()).when(mKeyStoreProxy).setEntry(

                anyString(),

                any(),

                any());



        assertThrows(InsecureUserException.class,

                () -> mPlatformKeyManager.getEncryptKey(USER_ID_FIXTURE));

    }



    @Test

    @Test

    public void getDecryptKey_generatesNewKeyIfOldOneIsInvalid() throws Exception {

    public void getDecryptKey_generatesNewKeyIfOldOneIsInvalid() throws Exception {

        doThrow(new UnrecoverableKeyException()).when(mKeyStoreProxy).getKey(

        doThrow(new UnrecoverableKeyException()).when(mKeyStoreProxy).getKey(