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

Commit a5d7857d authored by Eric Biggers's avatar Eric Biggers
Browse files

Use consistent helper class for keystore authorization 

Currently the IKeystoreAuthorization service is intended to be accessed
through the helper class android.security.Authorization.  However,
because Authorization provides only static methods, it can only be
unit-tested by static mocking, which is only available in
mockingservicestests.  BiometricService works around this in two
different ways: (a) using IKeystoreAuthorization directly, and (b) using
android.security.KeyStore, which is an obsolete class which is now
almost empty and just contains a couple random helpers.  I'd like to
remove it to avoid confusion with java.security.KeyStore.

This CL solves the testability problem in a consistent way by renaming
Authorization to KeyStoreAuthorization and changing all public static
methods to instance methods.  It updates all callers of the keystore
authorization service to go through a KeyStoreAuthorization instance.
Finally, it updates the unit tests for TrustManagerService and
BiometricService to inject a mock KeyStoreAuthorization.

Bug: 326508120
Test: atest TrustManagerServiceTest
Test: atest FrameworksServicesTests:{BiometricServiceTest,AuthSessionTest}
Test: atest CtsBiometricsTestCases:BiometricSimpleTests
Flag: N/A.  Refactoring with no behavior change intended.
Change-Id: I68504f447b1b880c08a60cf027b13f77a6567ec9
parent 4f490c58

keystore/java/android/security/KeyStore.java

+0 −14
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@ 
package android.security;



import android.compat.annotation.UnsupportedAppUsage;

import android.os.StrictMode;



/**

 * This class provides some constants and helper methods related to Android's Keystore service.
@@ -38,17 +37,4 @@ public class KeyStore { 
    public static KeyStore getInstance() {

        return KEY_STORE;

    }



    /**

     * Add an authentication record to the keystore authorization table.

     *

     * @param authToken The packed bytes of a hw_auth_token_t to be provided to keymaster.

     * @return 0 on success, otherwise an error value corresponding to a

     * {@code KeymasterDefs.KM_ERROR_} value or {@code KeyStore} ResponseCode.

     */

    public int addAuthToken(byte[] authToken) {

        StrictMode.noteDiskWrite();



        return Authorization.addAuthToken(authToken);

    }

}

keystore/java/android/security/Authorization.java→keystore/java/android/security/KeyStoreAuthorization.java

+19 −10
Original line number Diff line number Diff line
@@ -33,15 +33,21 @@ import android.util.Log; 
 * @hide This is the client side for IKeystoreAuthorization AIDL.

 * It shall only be used by biometric authentication providers and Gatekeeper.

 */

public class Authorization {

    private static final String TAG = "KeystoreAuthorization";

public class KeyStoreAuthorization {

    private static final String TAG = "KeyStoreAuthorization";



    public static final int SYSTEM_ERROR = ResponseCode.SYSTEM_ERROR;



    private static final KeyStoreAuthorization sInstance = new KeyStoreAuthorization();



    public static KeyStoreAuthorization getInstance() {

        return sInstance;

    }



    /**

     * @return an instance of IKeystoreAuthorization

     */

    public static IKeystoreAuthorization getService() {

    private IKeystoreAuthorization getService() {

        return IKeystoreAuthorization.Stub.asInterface(

                    ServiceManager.checkService("android.security.authorization"));

    }
@@ -52,7 +58,7 @@ public class Authorization { 
     * @param authToken created by Android authenticators.

     * @return 0 if successful or {@code ResponseCode.SYSTEM_ERROR}.

     */

    public static int addAuthToken(@NonNull HardwareAuthToken authToken) {

    public int addAuthToken(@NonNull HardwareAuthToken authToken) {

        StrictMode.noteSlowCall("addAuthToken");

        try {

            getService().addAuthToken(authToken);
@@ -70,7 +76,7 @@ public class Authorization { 
     * @param authToken

     * @return 0 if successful or a {@code ResponseCode}.

     */

    public static int addAuthToken(@NonNull byte[] authToken) {

    public int addAuthToken(@NonNull byte[] authToken) {

        return addAuthToken(AuthTokenUtils.toHardwareAuthToken(authToken));

    }
@@ -82,7 +88,7 @@ public class Authorization { 
     *                   is LSKF (or equivalent) and thus has made the synthetic password available

     * @return 0 if successful or a {@code ResponseCode}.

     */

    public static int onDeviceUnlocked(int userId, @Nullable byte[] password) {

    public int onDeviceUnlocked(int userId, @Nullable byte[] password) {

        StrictMode.noteDiskWrite();

        try {

            getService().onDeviceUnlocked(userId, password);
@@ -103,7 +109,7 @@ public class Authorization { 
     * @param weakUnlockEnabled - true if non-strong biometric or trust agent unlock is enabled

     * @return 0 if successful or a {@code ResponseCode}.

     */

    public static int onDeviceLocked(int userId, @NonNull long[] unlockingSids,

    public int onDeviceLocked(int userId, @NonNull long[] unlockingSids,

            boolean weakUnlockEnabled) {

        StrictMode.noteDiskWrite();

        try {
@@ -125,14 +131,17 @@ public class Authorization { 
     * @return the last authentication time or

     * {@link BiometricConstants#BIOMETRIC_NO_AUTHENTICATION}.

     */

    public static long getLastAuthenticationTime(

            long userId, @HardwareAuthenticatorType int[] authenticatorTypes) {

    public long getLastAuthTime(long userId, @HardwareAuthenticatorType int[] authenticatorTypes) {

        try {

            return getService().getLastAuthTime(userId, authenticatorTypes);

        } catch (RemoteException | NullPointerException e) {

            Log.w(TAG, "Can not connect to keystore", e);

            Log.w(TAG, "Error getting last auth time: " + e);

            return BiometricConstants.BIOMETRIC_NO_AUTHENTICATION;

        } catch (ServiceSpecificException e) {

            // This is returned when the feature flag test fails in keystore2

            if (e.errorCode == ResponseCode.PERMISSION_DENIED) {

                throw new UnsupportedOperationException();

            }

            return BiometricConstants.BIOMETRIC_NO_AUTHENTICATION;

        }

    }

services/core/java/com/android/server/biometrics/AuthSession.java

+9 −9
Original line number Diff line number Diff line
@@ -56,7 +56,7 @@ import android.hardware.fingerprint.FingerprintManager; 
import android.hardware.fingerprint.FingerprintSensorPropertiesInternal;

import android.os.IBinder;

import android.os.RemoteException;

import android.security.KeyStore;

import android.security.KeyStoreAuthorization;

import android.util.Slog;



import com.android.internal.annotations.VisibleForTesting;
@@ -111,7 +111,7 @@ public final class AuthSession implements IBinder.DeathRecipient { 
    @NonNull private final BiometricContext mBiometricContext;

    private final IStatusBarService mStatusBarService;

    @VisibleForTesting final IBiometricSysuiReceiver mSysuiReceiver;

    private final KeyStore mKeyStore;

    private final KeyStoreAuthorization mKeyStoreAuthorization;

    private final Random mRandom;

    private final ClientDeathReceiver mClientDeathReceiver;

    final PreAuthInfo mPreAuthInfo;
@@ -158,7 +158,7 @@ public final class AuthSession implements IBinder.DeathRecipient { 
            @NonNull BiometricContext biometricContext,

            @NonNull IStatusBarService statusBarService,

            @NonNull IBiometricSysuiReceiver sysuiReceiver,

            @NonNull KeyStore keystore,

            @NonNull KeyStoreAuthorization keyStoreAuthorization,

            @NonNull Random random,

            @NonNull ClientDeathReceiver clientDeathReceiver,

            @NonNull PreAuthInfo preAuthInfo,
@@ -172,8 +172,8 @@ public final class AuthSession implements IBinder.DeathRecipient { 
            @NonNull PromptInfo promptInfo,

            boolean debugEnabled,

            @NonNull List<FingerprintSensorPropertiesInternal> fingerprintSensorProperties) {

        this(context, biometricContext, statusBarService, sysuiReceiver, keystore, random,

                clientDeathReceiver, preAuthInfo, token, requestId, operationId, userId,

        this(context, biometricContext, statusBarService, sysuiReceiver, keyStoreAuthorization,

                random, clientDeathReceiver, preAuthInfo, token, requestId, operationId, userId,

                sensorReceiver, clientReceiver, opPackageName, promptInfo, debugEnabled,

                fingerprintSensorProperties, BiometricFrameworkStatsLogger.getInstance());

    }
@@ -183,7 +183,7 @@ public final class AuthSession implements IBinder.DeathRecipient { 
            @NonNull BiometricContext biometricContext,

            @NonNull IStatusBarService statusBarService,

            @NonNull IBiometricSysuiReceiver sysuiReceiver,

            @NonNull KeyStore keystore,

            @NonNull KeyStoreAuthorization keyStoreAuthorization,

            @NonNull Random random,

            @NonNull ClientDeathReceiver clientDeathReceiver,

            @NonNull PreAuthInfo preAuthInfo,
@@ -203,7 +203,7 @@ public final class AuthSession implements IBinder.DeathRecipient { 
        mBiometricContext = biometricContext;

        mStatusBarService = statusBarService;

        mSysuiReceiver = sysuiReceiver;

        mKeyStore = keystore;

        mKeyStoreAuthorization = keyStoreAuthorization;

        mRandom = random;

        mClientDeathReceiver = clientDeathReceiver;

        mPreAuthInfo = preAuthInfo;
@@ -814,14 +814,14 @@ public final class AuthSession implements IBinder.DeathRecipient { 
            switch (reason) {

                case BiometricPrompt.DISMISSED_REASON_CREDENTIAL_CONFIRMED:

                    if (credentialAttestation != null) {

                        mKeyStore.addAuthToken(credentialAttestation);

                        mKeyStoreAuthorization.addAuthToken(credentialAttestation);

                    } else {

                        Slog.e(TAG, "credentialAttestation is null");

                    }

                case BiometricPrompt.DISMISSED_REASON_BIOMETRIC_CONFIRMED:

                case BiometricPrompt.DISMISSED_REASON_BIOMETRIC_CONFIRM_NOT_REQUIRED:

                    if (mTokenEscrow != null) {

                        final int result = mKeyStore.addAuthToken(mTokenEscrow);

                        final int result = mKeyStoreAuthorization.addAuthToken(mTokenEscrow);

                        Slog.d(TAG, "addAuthToken: " + result);

                    } else {

                        Slog.e(TAG, "mTokenEscrow is null");

services/core/java/com/android/server/biometrics/BiometricService.java

+7 −30
Original line number Diff line number Diff line
@@ -65,15 +65,11 @@ import android.os.IBinder; 
import android.os.Looper;

import android.os.RemoteException;

import android.os.ServiceManager;

import android.os.ServiceSpecificException;

import android.os.UserHandle;

import android.os.UserManager;

import android.provider.Settings;

import android.security.Authorization;

import android.security.GateKeeper;

import android.security.KeyStore;

import android.security.authorization.IKeystoreAuthorization;

import android.security.authorization.ResponseCode;

import android.security.KeyStoreAuthorization;

import android.service.gatekeeper.IGateKeeperService;

import android.text.TextUtils;

import android.util.ArraySet;
@@ -123,11 +119,9 @@ public class BiometricService extends SystemService { 
    @VisibleForTesting

    IStatusBarService mStatusBarService;

    @VisibleForTesting

    KeyStore mKeyStore;

    @VisibleForTesting

    ITrustManager mTrustManager;

    @VisibleForTesting

    IKeystoreAuthorization mKeystoreAuthorization;

    KeyStoreAuthorization mKeyStoreAuthorization;

    @VisibleForTesting

    IGateKeeperService mGateKeeper;
@@ -672,19 +666,7 @@ public class BiometricService extends SystemService { 
            int[] authTypesArray = hardwareAuthenticators.stream()

                    .mapToInt(Integer::intValue)

                    .toArray();

            try {

                return mKeystoreAuthorization.getLastAuthTime(secureUserId, authTypesArray);

            } catch (RemoteException e) {

                Slog.w(TAG, "Error getting last auth time: " + e);

                return BiometricConstants.BIOMETRIC_NO_AUTHENTICATION;

            } catch (ServiceSpecificException e) {

                // This is returned when the feature flag test fails in keystore2

                if (e.errorCode == ResponseCode.PERMISSION_DENIED) {

                    throw new UnsupportedOperationException();

                }



                return BiometricConstants.BIOMETRIC_NO_AUTHENTICATION;

            }

            return mKeyStoreAuthorization.getLastAuthTime(secureUserId, authTypesArray);

        }



        @android.annotation.EnforcePermission(android.Manifest.permission.USE_BIOMETRIC_INTERNAL)
@@ -1009,8 +991,8 @@ public class BiometricService extends SystemService { 
            return ActivityManager.getService();

        }



        public IKeystoreAuthorization getKeystoreAuthorizationService() {

            return Authorization.getService();

        public KeyStoreAuthorization getKeyStoreAuthorization() {

            return KeyStoreAuthorization.getInstance();

        }



        public IGateKeeperService getGateKeeperService() {
@@ -1034,10 +1016,6 @@ public class BiometricService extends SystemService { 
            return new SettingObserver(context, handler, callbacks);

        }



        public KeyStore getKeyStore() {

            return KeyStore.getInstance();

        }



        /**

         * Allows to enable/disable debug logs.

         */
@@ -1130,7 +1108,7 @@ public class BiometricService extends SystemService { 
        mBiometricContext = injector.getBiometricContext(context);

        mUserManager = injector.getUserManager(context);

        mBiometricCameraManager = injector.getBiometricCameraManager(context);

        mKeystoreAuthorization = injector.getKeystoreAuthorizationService();

        mKeyStoreAuthorization = injector.getKeyStoreAuthorization();

        mGateKeeper = injector.getGateKeeperService();



        try {
@@ -1150,7 +1128,6 @@ public class BiometricService extends SystemService { 


    @Override

    public void onStart() {

        mKeyStore = mInjector.getKeyStore();

        mStatusBarService = mInjector.getStatusBarService();

        mTrustManager = mInjector.getTrustManager();

        mInjector.publishBinderService(this, mImpl);
@@ -1458,7 +1435,7 @@ public class BiometricService extends SystemService { 


        final boolean debugEnabled = mInjector.isDebugEnabled(getContext(), userId);

        mAuthSession = new AuthSession(getContext(), mBiometricContext, mStatusBarService,

                createSysuiReceiver(requestId), mKeyStore, mRandom,

                createSysuiReceiver(requestId), mKeyStoreAuthorization, mRandom,

                createClientDeathReceiver(requestId), preAuthInfo, token, requestId,

                operationId, userId, createBiometricSensorReceiver(requestId), receiver,

                opPackageName, promptInfo, debugEnabled,

services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java

+2 −2
Original line number Diff line number Diff line
@@ -30,7 +30,7 @@ import android.hardware.biometrics.BiometricManager; 
import android.hardware.biometrics.BiometricRequestConstants;

import android.os.IBinder;

import android.os.RemoteException;

import android.security.KeyStore;

import android.security.KeyStoreAuthorization;

import android.util.EventLog;

import android.util.Slog;
@@ -255,7 +255,7 @@ public abstract class AuthenticationClient<T, O extends AuthenticateOptions> 


            // For BP, BiometricService will add the authToken to Keystore.

            if (!isBiometricPrompt() && mIsStrongBiometric) {

                final int result = KeyStore.getInstance().addAuthToken(byteToken);

                final int result = KeyStoreAuthorization.getInstance().addAuthToken(byteToken);

                if (result != 0) {

                    Slog.d(TAG, "Error adding auth token : " + result);

                } else {