From fa8b98f9b7a696cb15e58ac29a71826138ec3acb Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Tue, 20 Jul 2021 14:12:40 -0700 Subject: [PATCH] 7/n: Add mechanism to hold onto already-completed operations The CoexCoordinator needs to hold onto successful operations in some cases, as well as prevent them from being "finished" in the scheduler (to prevent subsequent operations from starting while the coordination is not complete yet). This change: 0) Splits AuthenticationClient "state" and "callback" (e.g. client can be done with the sensor, but not notify the scheduler of completion yet). 1) Adds a list of successful operations. This list should usually be at most size 1 2) Each successful operation has an internal watchdog that removes itself from the list and finishes the AuthenticationClient's lifecycle, to prevent the scheduler from being stuck indefinitely 3) Adds support for additional coex cases. These should (I think) all be covered by newly added unit tests Test: atest com.android.server.biometrics Test: atest CoexCoordinatorTest (subset of the above) Bug: 193089985 Change-Id: I39a31a4e450001ab7db48055165c984a312b6bfa --- .../keyguard/KeyguardUpdateMonitor.java | 4 + .../sensors/AuthenticationClient.java | 53 ++++- .../sensors/BiometricScheduler.java | 1 - .../biometrics/sensors/CoexCoordinator.java | 192 +++++++++++++++++- .../face/aidl/FaceAuthenticationClient.java | 22 +- .../face/hidl/FaceAuthenticationClient.java | 22 +- .../aidl/FingerprintAuthenticationClient.java | 28 ++- .../fingerprint/hidl/Fingerprint21.java | 2 +- .../hidl/FingerprintAuthenticationClient.java | 31 ++- .../sensors/BiometricSchedulerTest.java | 10 + .../sensors/CoexCoordinatorTest.java | 147 +++++++++++++- 11 files changed, 475 insertions(+), 37 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java index e6e2ac980889a..2a0f9aead16f1 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java @@ -3315,6 +3315,7 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab pw.println(" trustManaged=" + getUserTrustIsManaged(userId)); pw.println(" udfpsEnrolled=" + isUdfpsEnrolled()); pw.println(" mFingerprintLockedOut=" + mFingerprintLockedOut); + pw.println(" mFingerprintLockedOutPermanent=" + mFingerprintLockedOutPermanent); pw.println(" enabledByUser=" + mBiometricEnabledForUser.get(userId)); if (isUdfpsEnrolled()) { pw.println(" shouldListenForUdfps=" + shouldListenForFingerprint(true)); @@ -3336,8 +3337,11 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab + getStrongAuthTracker().hasUserAuthenticatedSinceBoot()); pw.println(" disabled(DPM)=" + isFaceDisabled(userId)); pw.println(" possible=" + isUnlockWithFacePossible(userId)); + pw.println(" listening: actual=" + mFaceRunningState + + " expected=(" + (shouldListenForFace() ? 1 : 0)); pw.println(" strongAuthFlags=" + Integer.toHexString(strongAuthFlags)); pw.println(" trustManaged=" + getUserTrustIsManaged(userId)); + pw.println(" mFaceLockedOutPermanent=" + mFaceLockedOutPermanent); pw.println(" enabledByUser=" + mBiometricEnabledForUser.get(userId)); pw.println(" mSecureCameraLaunched=" + mSecureCameraLaunched); } diff --git a/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java index 28c949d4ed870..6463e04a4ff6c 100644 --- a/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java @@ -16,6 +16,7 @@ package com.android.server.biometrics.sensors; +import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.app.ActivityManager; @@ -30,6 +31,7 @@ import android.hardware.biometrics.BiometricManager; import android.hardware.biometrics.BiometricsProtoEnums; import android.os.IBinder; import android.os.RemoteException; +import android.os.SystemClock; import android.security.KeyStore; import android.util.EventLog; import android.util.Slog; @@ -48,6 +50,18 @@ public abstract class AuthenticationClient extends AcquisitionClient private static final String TAG = "Biometrics/AuthenticationClient"; + // New, has not started yet + public static final int STATE_NEW = 0; + // Framework/HAL have started this operation + public static final int STATE_STARTED = 1; + // Operation is started, but requires some user action (such as finger lift & re-touch) + public static final int STATE_STARTED_PAUSED = 2; + // Done, errored, canceled, etc. HAL/framework are not running this sensor anymore. + public static final int STATE_STOPPED = 3; + + @IntDef({STATE_NEW, STATE_STARTED, STATE_STARTED_PAUSED, STATE_STOPPED}) + @interface State {} + private final boolean mIsStrongBiometric; private final boolean mRequireConfirmation; private final ActivityTaskManager mActivityTaskManager; @@ -63,6 +77,20 @@ public abstract class AuthenticationClient extends AcquisitionClient protected boolean mAuthAttempted; + // TODO: This is currently hard to maintain, as each AuthenticationClient subclass must update + // the state. We should think of a way to improve this in the future. + protected @State int mState = STATE_NEW; + + /** + * Handles lifecycle, e.g. {@link BiometricScheduler}, + * {@link com.android.server.biometrics.sensors.BaseClientMonitor.Callback} after authentication + * results are known. Note that this happens asynchronously from (but shortly after) + * {@link #onAuthenticated(BiometricAuthenticator.Identifier, boolean, ArrayList)} and allows + * {@link CoexCoordinator} a chance to invoke/delay this event. + * @param authenticated + */ + protected abstract void handleLifecycleAfterAuth(boolean authenticated); + public AuthenticationClient(@NonNull Context context, @NonNull LazyDaemon lazyDaemon, @NonNull IBinder token, @NonNull ClientMonitorCallbackConverter listener, int targetUserId, long operationId, boolean restricted, @NonNull String owner, @@ -221,7 +249,8 @@ public abstract class AuthenticationClient extends AcquisitionClient } final CoexCoordinator coordinator = CoexCoordinator.getInstance(); - coordinator.onAuthenticationSucceeded(this, new CoexCoordinator.Callback() { + coordinator.onAuthenticationSucceeded(SystemClock.uptimeMillis(), this, + new CoexCoordinator.Callback() { @Override public void sendAuthenticationResult(boolean addAuthTokenIfStrong) { if (addAuthTokenIfStrong && mIsStrongBiometric) { @@ -262,6 +291,11 @@ public abstract class AuthenticationClient extends AcquisitionClient vibrateSuccess(); } } + + @Override + public void handleLifecycleAfterAuth() { + AuthenticationClient.this.handleLifecycleAfterAuth(true /* authenticated */); + } }); } else { // Allow system-defined limit of number of attempts before giving up @@ -272,7 +306,7 @@ public abstract class AuthenticationClient extends AcquisitionClient } final CoexCoordinator coordinator = CoexCoordinator.getInstance(); - coordinator.onAuthenticationRejected(this, lockoutMode, + coordinator.onAuthenticationRejected(SystemClock.uptimeMillis(), this, lockoutMode, new CoexCoordinator.Callback() { @Override public void sendAuthenticationResult(boolean addAuthTokenIfStrong) { @@ -291,6 +325,11 @@ public abstract class AuthenticationClient extends AcquisitionClient vibrateError(); } } + + @Override + public void handleLifecycleAfterAuth() { + AuthenticationClient.this.handleLifecycleAfterAuth(false /* authenticated */); + } }); } } @@ -307,6 +346,12 @@ public abstract class AuthenticationClient extends AcquisitionClient } } + @Override + public void onError(int errorCode, int vendorCode) { + super.onError(errorCode, vendorCode); + mState = STATE_STOPPED; + } + /** * Start authentication */ @@ -345,6 +390,10 @@ public abstract class AuthenticationClient extends AcquisitionClient } } + public @State int getState() { + return mState; + } + @Override public int getProtoEnum() { return BiometricsProto.CM_AUTHENTICATE; diff --git a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java index 1ac91672c7dd0..b20316e4c6df0 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java +++ b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java @@ -355,7 +355,6 @@ public class BiometricScheduler { /** * Creates a new scheduler. - * @param context system_server context. * @param tag for the specific instance of the scheduler. Should be unique. * @param sensorType the sensorType that this scheduler is handling. * @param gestureAvailabilityDispatcher may be null if the sensor does not support gestures diff --git a/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java b/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java index 7638a51a3710f..f97cb8a67d811 100644 --- a/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java +++ b/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java @@ -22,12 +22,17 @@ import static com.android.server.biometrics.sensors.BiometricScheduler.sensorTyp import android.annotation.NonNull; import android.annotation.Nullable; +import android.os.Handler; +import android.os.Looper; import android.util.Slog; +import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; +import com.android.server.biometrics.sensors.BiometricScheduler.SensorType; import com.android.server.biometrics.sensors.fingerprint.Udfps; import java.util.HashMap; +import java.util.LinkedList; import java.util.Map; /** @@ -43,6 +48,9 @@ public class CoexCoordinator { "com.android.server.biometrics.sensors.CoexCoordinator.enable"; private static final boolean DEBUG = true; + // Successful authentications should be used within this amount of time. + static final long SUCCESSFUL_AUTH_VALID_DURATION_MS = 5000; + /** * Callback interface notifying the owner of "results" from the CoexCoordinator's business * logic. @@ -58,10 +66,69 @@ public class CoexCoordinator { * Requests the owner to initiate a vibration for this event. */ void sendHapticFeedback(); + + /** + * Requests the owner to handle the AuthenticationClient's lifecycle (e.g. finish and remove + * from scheduler if auth was successful). + */ + void handleLifecycleAfterAuth(); } private static CoexCoordinator sInstance; + @VisibleForTesting + public static class SuccessfulAuth { + final long mAuthTimestamp; + final @SensorType int mSensorType; + final AuthenticationClient mAuthenticationClient; + final Callback mCallback; + final CleanupRunnable mCleanupRunnable; + + public static class CleanupRunnable implements Runnable { + @NonNull final LinkedList mSuccessfulAuths; + @NonNull final SuccessfulAuth mAuth; + @NonNull final Callback mCallback; + + public CleanupRunnable(@NonNull LinkedList successfulAuths, + @NonNull SuccessfulAuth auth, @NonNull Callback callback) { + mSuccessfulAuths = successfulAuths; + mAuth = auth; + mCallback = callback; + } + + @Override + public void run() { + final boolean removed = mSuccessfulAuths.remove(mAuth); + Slog.w(TAG, "Removing stale successfulAuth: " + mAuth.toString() + + ", success: " + removed); + mCallback.handleLifecycleAfterAuth(); + } + } + + public SuccessfulAuth(@NonNull Handler handler, + @NonNull LinkedList successfulAuths, + long currentTimeMillis, + @SensorType int sensorType, + @NonNull AuthenticationClient authenticationClient, + @NonNull Callback callback) { + mAuthTimestamp = currentTimeMillis; + mSensorType = sensorType; + mAuthenticationClient = authenticationClient; + mCallback = callback; + + mCleanupRunnable = new CleanupRunnable(successfulAuths, this, callback); + + handler.postDelayed(mCleanupRunnable, SUCCESSFUL_AUTH_VALID_DURATION_MS); + } + + @Override + public String toString() { + return "SensorType: " + sensorTypeToString(mSensorType) + + ", mAuthTimestamp: " + mAuthTimestamp + + ", authenticationClient: " + mAuthenticationClient; + } + } + /** * @return a singleton instance. */ @@ -85,11 +152,15 @@ public class CoexCoordinator { // SensorType to AuthenticationClient map private final Map> mClientMap; + @VisibleForTesting final LinkedList mSuccessfulAuths; private boolean mAdvancedLogicEnabled; + private final Handler mHandler; private CoexCoordinator() { // Singleton mClientMap = new HashMap<>(); + mSuccessfulAuths = new LinkedList<>(); + mHandler = new Handler(Looper.getMainLooper()); } public void addAuthenticationClient(@BiometricScheduler.SensorType int sensorType, @@ -121,34 +192,43 @@ public class CoexCoordinator { mClientMap.remove(sensorType); } - public void onAuthenticationSucceeded(@NonNull AuthenticationClient client, + public void onAuthenticationSucceeded(long currentTimeMillis, + @NonNull AuthenticationClient client, @NonNull Callback callback) { if (client.isBiometricPrompt()) { callback.sendHapticFeedback(); // For BP, BiometricService will add the authToken to Keystore. callback.sendAuthenticationResult(false /* addAuthTokenIfStrong */); + callback.handleLifecycleAfterAuth(); } else if (isUnknownClient(client)) { // Client doesn't exist in our map for some reason. Give the user feedback so the // device doesn't feel like it's stuck. All other cases below can assume that the // client exists in our map. callback.sendHapticFeedback(); callback.sendAuthenticationResult(true /* addAuthTokenIfStrong */); + callback.handleLifecycleAfterAuth(); } else if (mAdvancedLogicEnabled && client.isKeyguard()) { if (isSingleAuthOnly(client)) { // Single sensor authentication callback.sendHapticFeedback(); callback.sendAuthenticationResult(true /* addAuthTokenIfStrong */); + callback.handleLifecycleAfterAuth(); } else { // Multi sensor authentication AuthenticationClient udfps = mClientMap.getOrDefault(SENSOR_TYPE_UDFPS, null); AuthenticationClient face = mClientMap.getOrDefault(SENSOR_TYPE_FACE, null); if (isCurrentFaceAuth(client)) { - if (isPointerDown(udfps)) { - // Face auth success while UDFPS pointer down. No callback, no haptic. - // Feedback will be provided after UDFPS result. + if (isUdfpsActivelyAuthing(udfps)) { + // Face auth success while UDFPS is actively authing. No callback, no haptic + // Feedback will be provided after UDFPS result: + // 1) UDFPS succeeds - simply remove this from the queue + // 2) UDFPS rejected - use this face auth success to notify clients + mSuccessfulAuths.add(new SuccessfulAuth(mHandler, mSuccessfulAuths, + currentTimeMillis, SENSOR_TYPE_FACE, client, callback)); } else { callback.sendHapticFeedback(); callback.sendAuthenticationResult(true /* addAuthTokenIfStrong */); + callback.handleLifecycleAfterAuth(); } } else if (isCurrentUdfps(client)) { if (isFaceScanning()) { @@ -156,8 +236,12 @@ public class CoexCoordinator { // Cancel face auth and/or prevent it from invoking haptics/callbacks after face.cancel(); } + + removeAndFinishAllFaceFromQueue(); + callback.sendHapticFeedback(); callback.sendAuthenticationResult(true /* addAuthTokenIfStrong */); + callback.handleLifecycleAfterAuth(); } } } else { @@ -165,13 +249,68 @@ public class CoexCoordinator { // FingerprintManager for highlighting fingers callback.sendHapticFeedback(); callback.sendAuthenticationResult(true /* addAuthTokenIfStrong */); + callback.handleLifecycleAfterAuth(); } } - public void onAuthenticationRejected(@NonNull AuthenticationClient client, + public void onAuthenticationRejected(long currentTimeMillis, + @NonNull AuthenticationClient client, @LockoutTracker.LockoutMode int lockoutMode, @NonNull Callback callback) { - callback.sendHapticFeedback(); + final boolean keyguardAdvancedLogic = mAdvancedLogicEnabled && client.isKeyguard(); + + if (keyguardAdvancedLogic) { + if (isSingleAuthOnly(client)) { + callback.sendHapticFeedback(); + callback.handleLifecycleAfterAuth(); + } else { + // Multi sensor authentication + AuthenticationClient udfps = mClientMap.getOrDefault(SENSOR_TYPE_UDFPS, null); + AuthenticationClient face = mClientMap.getOrDefault(SENSOR_TYPE_FACE, null); + if (isCurrentFaceAuth(client)) { + // UDFPS should still be running in this case, do not vibrate. However, we + // should notify the callback and finish the client, so that Keyguard and + // BiometricScheduler do not get stuck. + Slog.d(TAG, "Face rejected in multi-sensor auth, udfps: " + udfps); + callback.handleLifecycleAfterAuth(); + } else if (isCurrentUdfps(client)) { + // Face should either be running, or have already finished + SuccessfulAuth auth = popSuccessfulFaceAuthIfExists(currentTimeMillis); + if (auth != null) { + Slog.d(TAG, "Using recent auth: " + auth); + callback.handleLifecycleAfterAuth(); + + auth.mCallback.sendHapticFeedback(); + auth.mCallback.sendAuthenticationResult(true /* addAuthTokenIfStrong */); + auth.mCallback.handleLifecycleAfterAuth(); + } else if (isFaceScanning()) { + // UDFPS rejected but face is still scanning + Slog.d(TAG, "UDFPS rejected in multi-sensor auth, face: " + face); + callback.handleLifecycleAfterAuth(); + + // TODO(b/193089985): Enforce/ensure that face auth finishes (whether + // accept/reject) within X amount of time. Otherwise users will be stuck + // waiting with their finger down for a long time. + } else { + // Face not scanning, and was not found in the queue. Most likely, face + // auth was too long ago. + Slog.d(TAG, "UDFPS rejected in multi-sensor auth, face not scanning"); + callback.sendHapticFeedback(); + callback.handleLifecycleAfterAuth(); + } + } else { + Slog.d(TAG, "Unknown client rejected: " + client); + callback.sendHapticFeedback(); + callback.handleLifecycleAfterAuth(); + } + } + } else { + callback.sendHapticFeedback(); + callback.handleLifecycleAfterAuth(); + } + + // Always notify keyguard, otherwise the cached "running" state in KeyguardUpdateMonitor + // will get stuck. if (lockoutMode == LockoutTracker.LOCKOUT_NONE) { // Don't send onAuthenticationFailed if we're in lockout, it causes a // janky UI on Keyguard/BiometricPrompt since "authentication failed" @@ -180,6 +319,30 @@ public class CoexCoordinator { } } + @Nullable + private SuccessfulAuth popSuccessfulFaceAuthIfExists(long currentTimeMillis) { + for (SuccessfulAuth auth : mSuccessfulAuths) { + if (currentTimeMillis - auth.mAuthTimestamp >= SUCCESSFUL_AUTH_VALID_DURATION_MS) { + Slog.d(TAG, "Removing stale auth: " + auth); + mSuccessfulAuths.remove(auth); + } else if (auth.mSensorType == SENSOR_TYPE_FACE) { + mSuccessfulAuths.remove(auth); + return auth; + } + } + return null; + } + + private void removeAndFinishAllFaceFromQueue() { + for (SuccessfulAuth auth : mSuccessfulAuths) { + if (auth.mSensorType == SENSOR_TYPE_FACE) { + Slog.d(TAG, "Removing from queue and finishing: " + auth); + auth.mCallback.handleLifecycleAfterAuth(); + mSuccessfulAuths.remove(auth); + } + } + } + private boolean isCurrentFaceAuth(@NonNull AuthenticationClient client) { return client == mClientMap.getOrDefault(SENSOR_TYPE_FACE, null); } @@ -189,12 +352,13 @@ public class CoexCoordinator { } private boolean isFaceScanning() { - return mClientMap.containsKey(SENSOR_TYPE_FACE); + AuthenticationClient client = mClientMap.getOrDefault(SENSOR_TYPE_FACE, null); + return client != null && client.getState() == AuthenticationClient.STATE_STARTED; } - private static boolean isPointerDown(@Nullable AuthenticationClient client) { + private static boolean isUdfpsActivelyAuthing(@Nullable AuthenticationClient client) { if (client instanceof Udfps) { - return ((Udfps) client).isPointerDown(); + return client.getState() == AuthenticationClient.STATE_STARTED; } return false; } @@ -221,7 +385,15 @@ public class CoexCoordinator { return true; } + @Override public String toString() { - return "Enabled: " + mAdvancedLogicEnabled; + StringBuilder sb = new StringBuilder(); + sb.append("Enabled: ").append(mAdvancedLogicEnabled); + sb.append(", Queue size: " ).append(mSuccessfulAuths.size()); + for (SuccessfulAuth auth : mSuccessfulAuths) { + sb.append(", Auth: ").append(auth.toString()); + } + + return sb.toString(); } } diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceAuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceAuthenticationClient.java index 0525d2da69880..35c17459804d9 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceAuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceAuthenticationClient.java @@ -89,6 +89,12 @@ class FaceAuthenticationClient extends AuthenticationClient implements R.array.config_face_acquire_vendor_keyguard_ignorelist); } + @Override + public void start(@NonNull Callback callback) { + super.start(callback); + mState = STATE_STARTED; + } + @NonNull @Override protected Callback wrapCallbackForStart(@NonNull Callback callback) { @@ -127,11 +133,21 @@ class FaceAuthenticationClient extends AuthenticationClient implements && mLastAcquire != FaceManager.FACE_ACQUIRED_UNKNOWN; } + @Override + protected void handleLifecycleAfterAuth(boolean authenticated) { + // For face, the authentication lifecycle ends either when + // 1) Authenticated == true + // 2) Error occurred + // 3) Authenticated == false + mCallback.onClientFinished(this, true /* success */); + } + @Override public void onAuthenticated(BiometricAuthenticator.Identifier identifier, boolean authenticated, ArrayList token) { super.onAuthenticated(identifier, authenticated, token); + mState = STATE_STOPPED; mUsageStats.addEvent(new UsageStats.AuthenticationEvent( getStartTimeMs(), System.currentTimeMillis() - getStartTimeMs() /* latency */, @@ -139,12 +155,6 @@ class FaceAuthenticationClient extends AuthenticationClient implements 0 /* error */, 0 /* vendorError */, getTargetUserId())); - - // For face, the authentication lifecycle ends either when - // 1) Authenticated == true - // 2) Error occurred - // 3) Authenticated == false - mCallback.onClientFinished(this, true /* success */); } @Override diff --git a/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceAuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceAuthenticationClient.java index 5731d73dfd492..e65245b98829e 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceAuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/hidl/FaceAuthenticationClient.java @@ -79,6 +79,12 @@ class FaceAuthenticationClient extends AuthenticationClient { R.array.config_face_acquire_vendor_keyguard_ignorelist); } + @Override + public void start(@NonNull Callback callback) { + super.start(callback); + mState = STATE_STARTED; + } + @NonNull @Override protected Callback wrapCallbackForStart(@NonNull Callback callback) { @@ -114,11 +120,21 @@ class FaceAuthenticationClient extends AuthenticationClient { && mLastAcquire != FaceManager.FACE_ACQUIRED_SENSOR_DIRTY; } + @Override + protected void handleLifecycleAfterAuth(boolean authenticated) { + // For face, the authentication lifecycle ends either when + // 1) Authenticated == true + // 2) Error occurred + // 3) Authenticated == false + mCallback.onClientFinished(this, true /* success */); + } + @Override public void onAuthenticated(BiometricAuthenticator.Identifier identifier, boolean authenticated, ArrayList token) { super.onAuthenticated(identifier, authenticated, token); + mState = STATE_STOPPED; mUsageStats.addEvent(new UsageStats.AuthenticationEvent( getStartTimeMs(), System.currentTimeMillis() - getStartTimeMs() /* latency */, @@ -126,12 +142,6 @@ class FaceAuthenticationClient extends AuthenticationClient { 0 /* error */, 0 /* vendorError */, getTargetUserId())); - - // For face, the authentication lifecycle ends either when - // 1) Authenticated == true - // 2) Error occurred - // 3) Authenticated == false - mCallback.onClientFinished(this, true /* success */); } @Override diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintAuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintAuthenticationClient.java index 639814bf549fc..14d18225d6740 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintAuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintAuthenticationClient.java @@ -54,6 +54,8 @@ class FingerprintAuthenticationClient extends AuthenticationClient imp @NonNull private final LockoutCache mLockoutCache; @Nullable private final IUdfpsOverlayController mUdfpsOverlayController; + @NonNull private final FingerprintSensorPropertiesInternal mSensorProps; + @Nullable private ICancellationSignal mCancellationSignal; private boolean mIsPointerDown; @@ -72,6 +74,19 @@ class FingerprintAuthenticationClient extends AuthenticationClient imp lockoutCache, allowBackgroundAuthentication, true /* shouldVibrate */); mLockoutCache = lockoutCache; mUdfpsOverlayController = udfpsOverlayController; + mSensorProps = sensorProps; + } + + @Override + public void start(@NonNull Callback callback) { + super.start(callback); + + if (mSensorProps.isAnyUdfpsType()) { + // UDFPS requires user to touch before becoming "active" + mState = STATE_STARTED_PAUSED; + } else { + mState = STATE_STARTED; + } } @NonNull @@ -80,14 +95,23 @@ class FingerprintAuthenticationClient extends AuthenticationClient imp return new CompositeCallback(createALSCallback(), callback); } + @Override + protected void handleLifecycleAfterAuth(boolean authenticated) { + if (authenticated) { + mCallback.onClientFinished(this, true /* success */); + } + } + @Override public void onAuthenticated(BiometricAuthenticator.Identifier identifier, boolean authenticated, ArrayList token) { super.onAuthenticated(identifier, authenticated, token); if (authenticated) { + mState = STATE_STOPPED; UdfpsHelper.hideUdfpsOverlay(getSensorId(), mUdfpsOverlayController); - mCallback.onClientFinished(this, true /* success */); + } else { + mState = STATE_STARTED_PAUSED; } } @@ -145,6 +169,7 @@ class FingerprintAuthenticationClient extends AuthenticationClient imp public void onPointerDown(int x, int y, float minor, float major) { try { mIsPointerDown = true; + mState = STATE_STARTED; getFreshDaemon().onPointerDown(0 /* pointerId */, x, y, minor, major); if (getListener() != null) { getListener().onUdfpsPointerDown(getSensorId()); @@ -158,6 +183,7 @@ class FingerprintAuthenticationClient extends AuthenticationClient imp public void onPointerUp() { try { mIsPointerDown = false; + mState = STATE_STARTED_PAUSED; getFreshDaemon().onPointerUp(0 /* pointerId */); if (getListener() != null) { getListener().onUdfpsPointerUp(getSensorId()); diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java index a6385a541b031..2f5b5c7b97270 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java @@ -622,7 +622,7 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider opPackageName, cookie, false /* requireConfirmation */, mSensorProperties.sensorId, isStrongBiometric, statsClient, mTaskStackListener, mLockoutTracker, mUdfpsOverlayController, - allowBackgroundAuthentication); + allowBackgroundAuthentication, mSensorProperties); mScheduler.scheduleClientMonitor(client, fingerprintStateCallback); }); } diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintAuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintAuthenticationClient.java index 95a54d3591a38..9347244d7c777 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintAuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/FingerprintAuthenticationClient.java @@ -25,6 +25,7 @@ import android.hardware.biometrics.BiometricConstants; import android.hardware.biometrics.BiometricFingerprintConstants; import android.hardware.biometrics.BiometricsProtoEnums; import android.hardware.biometrics.fingerprint.V2_1.IBiometricsFingerprint; +import android.hardware.fingerprint.FingerprintSensorPropertiesInternal; import android.hardware.fingerprint.IUdfpsOverlayController; import android.os.IBinder; import android.os.RemoteException; @@ -52,6 +53,8 @@ class FingerprintAuthenticationClient extends AuthenticationClient { @@ -395,6 +400,11 @@ public class BiometricSchedulerTest { protected void startHalOperation() { } + + @Override + protected void handleLifecycleAfterAuth(boolean authenticated) { + + } } private static class TestClientMonitor2 extends TestClientMonitor { diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/CoexCoordinatorTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/CoexCoordinatorTest.java index 9c42f45584508..fb05825a122b3 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/CoexCoordinatorTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/CoexCoordinatorTest.java @@ -19,6 +19,9 @@ package com.android.server.biometrics.sensors; import static com.android.server.biometrics.sensors.BiometricScheduler.SENSOR_TYPE_FACE; import static com.android.server.biometrics.sensors.BiometricScheduler.SENSOR_TYPE_UDFPS; +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertTrue; + import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -28,8 +31,11 @@ import static org.mockito.Mockito.when; import static org.mockito.Mockito.withSettings; import android.content.Context; +import android.os.Handler; +import android.os.Looper; import android.platform.test.annotations.Presubmit; +import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; import com.android.server.biometrics.sensors.fingerprint.Udfps; @@ -39,6 +45,8 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.LinkedList; + @Presubmit @SmallTest public class CoexCoordinatorTest { @@ -46,6 +54,7 @@ public class CoexCoordinatorTest { private static final String TAG = "CoexCoordinatorTest"; private CoexCoordinator mCoexCoordinator; + private Handler mHandler; @Mock private Context mContext; @@ -55,6 +64,9 @@ public class CoexCoordinatorTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); + + mHandler = new Handler(Looper.getMainLooper()); + mCoexCoordinator = CoexCoordinator.getInstance(); mCoexCoordinator.setAdvancedLogicEnabled(true); } @@ -68,9 +80,10 @@ public class CoexCoordinatorTest { mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, client); - mCoexCoordinator.onAuthenticationSucceeded(client, mCallback); + mCoexCoordinator.onAuthenticationSucceeded(0 /* currentTimeMillis */, client, mCallback); verify(mCallback).sendHapticFeedback(); verify(mCallback).sendAuthenticationResult(eq(false) /* addAuthTokenIfStrong */); + verify(mCallback).handleLifecycleAfterAuth(); } @Test @@ -82,9 +95,11 @@ public class CoexCoordinatorTest { mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, client); - mCoexCoordinator.onAuthenticationRejected(client, LockoutTracker.LOCKOUT_NONE, mCallback); + mCoexCoordinator.onAuthenticationRejected(0 /* currentTimeMillis */, + client, LockoutTracker.LOCKOUT_NONE, mCallback); verify(mCallback).sendHapticFeedback(); verify(mCallback).sendAuthenticationResult(eq(false) /* addAuthTokenIfStrong */); + verify(mCallback).handleLifecycleAfterAuth(); } @Test @@ -96,9 +111,11 @@ public class CoexCoordinatorTest { mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, client); - mCoexCoordinator.onAuthenticationRejected(client, LockoutTracker.LOCKOUT_TIMED, mCallback); + mCoexCoordinator.onAuthenticationRejected(0 /* currentTimeMillis */, + client, LockoutTracker.LOCKOUT_TIMED, mCallback); verify(mCallback).sendHapticFeedback(); verify(mCallback, never()).sendAuthenticationResult(anyBoolean()); + verify(mCallback).handleLifecycleAfterAuth(); } @Test @@ -110,9 +127,10 @@ public class CoexCoordinatorTest { mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, client); - mCoexCoordinator.onAuthenticationSucceeded(client, mCallback); + mCoexCoordinator.onAuthenticationSucceeded(0 /* currentTimeMillis */, client, mCallback); verify(mCallback).sendHapticFeedback(); verify(mCallback).sendAuthenticationResult(eq(true) /* addAuthTokenIfStrong */); + verify(mCallback).handleLifecycleAfterAuth(); } @Test @@ -130,13 +148,33 @@ public class CoexCoordinatorTest { mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, faceClient); mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_UDFPS, udfpsClient); - mCoexCoordinator.onAuthenticationSucceeded(faceClient, mCallback); + mCoexCoordinator.onAuthenticationSucceeded(0 /* currentTimeMillis */, faceClient, + mCallback); verify(mCallback).sendHapticFeedback(); verify(mCallback).sendAuthenticationResult(eq(true) /* addAuthTokenIfStrong */); + verify(mCallback).handleLifecycleAfterAuth(); } @Test - public void testKeyguard_faceAuth_udfpsTouching_faceSuccess() { + public void testKeyguard_faceAuth_udfpsTouching_faceSuccess_thenUdfpsRejectedWithinBounds() { + testKeyguard_faceAuth_udfpsTouching_faceSuccess(false /* thenUdfpsAccepted */, + 0 /* udfpsRejectedAfterMs */); + } + + @Test + public void testKeyguard_faceAuth_udfpsTouching_faceSuccess_thenUdfpsRejectedAfterBounds() { + testKeyguard_faceAuth_udfpsTouching_faceSuccess(false /* thenUdfpsAccepted */, + CoexCoordinator.SUCCESSFUL_AUTH_VALID_DURATION_MS + 1 /* udfpsRejectedAfterMs */); + } + + @Test + public void testKeyguard_faceAuth_udfpsTouching_faceSuccess_thenUdfpsAccepted() { + testKeyguard_faceAuth_udfpsTouching_faceSuccess(true /* thenUdfpsAccepted */, + 0 /* udfpsRejectedAfterMs */); + } + + private void testKeyguard_faceAuth_udfpsTouching_faceSuccess(boolean thenUdfpsAccepted, + long udfpsRejectedAfterMs) { mCoexCoordinator.reset(); AuthenticationClient faceClient = mock(AuthenticationClient.class); @@ -146,13 +184,54 @@ public class CoexCoordinatorTest { withSettings().extraInterfaces(Udfps.class)); when(udfpsClient.isKeyguard()).thenReturn(true); when(((Udfps) udfpsClient).isPointerDown()).thenReturn(true); + when (udfpsClient.getState()).thenReturn(AuthenticationClient.STATE_STARTED); mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, faceClient); mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_UDFPS, udfpsClient); - mCoexCoordinator.onAuthenticationSucceeded(faceClient, mCallback); + mCoexCoordinator.onAuthenticationSucceeded(0 /* currentTimeMillis */, faceClient, + mCallback); verify(mCallback, never()).sendHapticFeedback(); verify(mCallback, never()).sendAuthenticationResult(anyBoolean()); + // CoexCoordinator requests the system to hold onto this AuthenticationClient until + // UDFPS result is known + verify(mCallback, never()).handleLifecycleAfterAuth(); + + // Reset the mock + CoexCoordinator.Callback udfpsCallback = mock(CoexCoordinator.Callback.class); + assertEquals(1, mCoexCoordinator.mSuccessfulAuths.size()); + assertEquals(faceClient, mCoexCoordinator.mSuccessfulAuths.get(0).mAuthenticationClient); + if (thenUdfpsAccepted) { + mCoexCoordinator.onAuthenticationSucceeded(0 /* currentTimeMillis */, udfpsClient, + udfpsCallback); + verify(udfpsCallback).sendHapticFeedback(); + verify(udfpsCallback).sendAuthenticationResult(true /* addAuthTokenIfStrong */); + verify(udfpsCallback).handleLifecycleAfterAuth(); + + assertTrue(mCoexCoordinator.mSuccessfulAuths.isEmpty()); + } else { + mCoexCoordinator.onAuthenticationRejected(udfpsRejectedAfterMs, udfpsClient, + LockoutTracker.LOCKOUT_NONE, udfpsCallback); + if (udfpsRejectedAfterMs <= CoexCoordinator.SUCCESSFUL_AUTH_VALID_DURATION_MS) { + verify(udfpsCallback, never()).sendHapticFeedback(); + + verify(mCallback).sendHapticFeedback(); + verify(mCallback).sendAuthenticationResult(eq(true) /* addAuthTokenIfStrong */); + verify(mCallback).handleLifecycleAfterAuth(); + + assertTrue(mCoexCoordinator.mSuccessfulAuths.isEmpty()); + } else { + assertTrue(mCoexCoordinator.mSuccessfulAuths.isEmpty()); + + verify(mCallback, never()).sendHapticFeedback(); + verify(mCallback, never()).sendAuthenticationResult(anyBoolean()); + + verify(udfpsCallback).sendHapticFeedback(); + verify(udfpsCallback) + .sendAuthenticationResult(eq(false) /* addAuthTokenIfStrong */); + verify(udfpsCallback).handleLifecycleAfterAuth(); + } + } } @Test @@ -161,6 +240,7 @@ public class CoexCoordinatorTest { AuthenticationClient faceClient = mock(AuthenticationClient.class); when(faceClient.isKeyguard()).thenReturn(true); + when(faceClient.getState()).thenReturn(AuthenticationClient.STATE_STARTED); AuthenticationClient udfpsClient = mock(AuthenticationClient.class, withSettings().extraInterfaces(Udfps.class)); @@ -170,9 +250,60 @@ public class CoexCoordinatorTest { mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, faceClient); mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_UDFPS, udfpsClient); - mCoexCoordinator.onAuthenticationSucceeded(udfpsClient, mCallback); + mCoexCoordinator.onAuthenticationSucceeded(0 /* currentTimeMillis */, udfpsClient, + mCallback); verify(mCallback).sendHapticFeedback(); verify(mCallback).sendAuthenticationResult(eq(true)); verify(faceClient).cancel(); + verify(mCallback).handleLifecycleAfterAuth(); + } + + @Test + public void testNonKeyguard_rejectAndNotLockedOut() { + mCoexCoordinator.reset(); + + AuthenticationClient faceClient = mock(AuthenticationClient.class); + when(faceClient.isKeyguard()).thenReturn(false); + when(faceClient.isBiometricPrompt()).thenReturn(true); + + mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, faceClient); + mCoexCoordinator.onAuthenticationRejected(0 /* currentTimeMillis */, faceClient, + LockoutTracker.LOCKOUT_NONE, mCallback); + + verify(mCallback).sendHapticFeedback(); + verify(mCallback).sendAuthenticationResult(eq(false)); + verify(mCallback).handleLifecycleAfterAuth(); + } + + @Test + public void testNonKeyguard_rejectLockedOut() { + mCoexCoordinator.reset(); + + AuthenticationClient faceClient = mock(AuthenticationClient.class); + when(faceClient.isKeyguard()).thenReturn(false); + when(faceClient.isBiometricPrompt()).thenReturn(true); + + mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, faceClient); + mCoexCoordinator.onAuthenticationRejected(0 /* currentTimeMillis */, faceClient, + LockoutTracker.LOCKOUT_TIMED, mCallback); + + verify(mCallback).sendHapticFeedback(); + verify(mCallback, never()).sendAuthenticationResult(anyBoolean()); + verify(mCallback).handleLifecycleAfterAuth(); + } + + @Test + public void testCleanupRunnable() { + LinkedList successfulAuths = mock(LinkedList.class); + CoexCoordinator.SuccessfulAuth auth = mock(CoexCoordinator.SuccessfulAuth.class); + CoexCoordinator.Callback callback = mock(CoexCoordinator.Callback.class); + CoexCoordinator.SuccessfulAuth.CleanupRunnable runnable = + new CoexCoordinator.SuccessfulAuth.CleanupRunnable(successfulAuths, auth, callback); + runnable.run(); + + InstrumentationRegistry.getInstrumentation().waitForIdleSync(); + + verify(callback).handleLifecycleAfterAuth(); + verify(successfulAuths).remove(eq(auth)); } }