diff options
| author | 2021-07-20 14:12:40 -0700 | |
|---|---|---|
| committer | 2021-07-21 13:25:59 -0700 | |
| commit | fa8b98f9b7a696cb15e58ac29a71826138ec3acb (patch) | |
| tree | 1773bddeb0008f91a142df4dfab1c41f044de443 | |
| parent | 9affacd3f8ce2d5352086fd09a166204421cd0b2 (diff) | |
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
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 e6e2ac980889..2a0f9aead16f 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 28c949d4ed87..6463e04a4ff6 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<T> extends AcquisitionClient<T> 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<T> extends AcquisitionClient<T> 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<T> lazyDaemon, @NonNull IBinder token, @NonNull ClientMonitorCallbackConverter listener, int targetUserId, long operationId, boolean restricted, @NonNull String owner, @@ -221,7 +249,8 @@ public abstract class AuthenticationClient<T> extends AcquisitionClient<T> } 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<T> extends AcquisitionClient<T> 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<T> extends AcquisitionClient<T> } 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<T> extends AcquisitionClient<T> vibrateError(); } } + + @Override + public void handleLifecycleAfterAuth() { + AuthenticationClient.this.handleLifecycleAfterAuth(false /* authenticated */); + } }); } } @@ -307,6 +346,12 @@ public abstract class AuthenticationClient<T> extends AcquisitionClient<T> } } + @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<T> extends AcquisitionClient<T> } } + 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 1ac91672c7dd..b20316e4c6df 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 7638a51a3710..f97cb8a67d81 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<SuccessfulAuth> mSuccessfulAuths; + @NonNull final SuccessfulAuth mAuth; + @NonNull final Callback mCallback; + + public CleanupRunnable(@NonNull LinkedList<SuccessfulAuth> 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<SuccessfulAuth> 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<Integer, AuthenticationClient<?>> mClientMap; + @VisibleForTesting final LinkedList<SuccessfulAuth> 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 0525d2da6988..35c17459804d 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<ISession> 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) { @@ -128,10 +134,20 @@ class FaceAuthenticationClient extends AuthenticationClient<ISession> implements } @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<Byte> 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<ISession> 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 5731d73dfd49..e65245b98829 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<IBiometricsFace> { 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) { @@ -115,10 +121,20 @@ class FaceAuthenticationClient extends AuthenticationClient<IBiometricsFace> { } @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<Byte> 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<IBiometricsFace> { 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 639814bf549f..14d18225d674 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<ISession> 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<ISession> 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 @@ -81,13 +96,22 @@ class FingerprintAuthenticationClient extends AuthenticationClient<ISession> imp } @Override + protected void handleLifecycleAfterAuth(boolean authenticated) { + if (authenticated) { + mCallback.onClientFinished(this, true /* success */); + } + } + + @Override public void onAuthenticated(BiometricAuthenticator.Identifier identifier, boolean authenticated, ArrayList<Byte> 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<ISession> 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<ISession> 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 a6385a541b03..2f5b5c7b9727 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 95a54d3591a3..9347244d7c77 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<IBiometricsFi private final LockoutFrameworkImpl mLockoutFrameworkImpl; @Nullable private final IUdfpsOverlayController mUdfpsOverlayController; + @NonNull private final FingerprintSensorPropertiesInternal mSensorProps; + private boolean mIsPointerDown; FingerprintAuthenticationClient(@NonNull Context context, @@ -62,13 +65,27 @@ class FingerprintAuthenticationClient extends AuthenticationClient<IBiometricsFi @NonNull TaskStackListener taskStackListener, @NonNull LockoutFrameworkImpl lockoutTracker, @Nullable IUdfpsOverlayController udfpsOverlayController, - boolean allowBackgroundAuthentication) { + boolean allowBackgroundAuthentication, + @NonNull FingerprintSensorPropertiesInternal sensorProps) { super(context, lazyDaemon, token, listener, targetUserId, operationId, restricted, owner, cookie, requireConfirmation, sensorId, isStrongBiometric, BiometricsProtoEnums.MODALITY_FINGERPRINT, statsClient, taskStackListener, lockoutTracker, allowBackgroundAuthentication, true /* shouldVibrate */); mLockoutFrameworkImpl = lockoutTracker; 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 @@ -88,10 +105,11 @@ class FingerprintAuthenticationClient extends AuthenticationClient<IBiometricsFi // Note that authentication doesn't end when Authenticated == false if (authenticated) { + mState = STATE_STOPPED; resetFailedAttempts(getTargetUserId()); UdfpsHelper.hideUdfpsOverlay(getSensorId(), mUdfpsOverlayController); - mCallback.onClientFinished(this, true /* success */); } else { + mState = STATE_STARTED_PAUSED; final @LockoutTracker.LockoutMode int lockoutMode = mLockoutFrameworkImpl.getLockoutModeForUser(getTargetUserId()); if (lockoutMode != LockoutTracker.LOCKOUT_NONE) { @@ -125,6 +143,13 @@ class FingerprintAuthenticationClient extends AuthenticationClient<IBiometricsFi } @Override + protected void handleLifecycleAfterAuth(boolean authenticated) { + if (authenticated) { + mCallback.onClientFinished(this, true /* success */); + } + } + + @Override public @LockoutTracker.LockoutMode int handleFailedAttempt(int userId) { mLockoutFrameworkImpl.addFailedAttemptForUser(userId); return super.handleFailedAttempt(userId); @@ -162,6 +187,7 @@ class FingerprintAuthenticationClient extends AuthenticationClient<IBiometricsFi @Override public void onPointerDown(int x, int y, float minor, float major) { mIsPointerDown = true; + mState = STATE_STARTED; UdfpsHelper.onFingerDown(getFreshDaemon(), x, y, minor, major); if (getListener() != null) { try { @@ -175,6 +201,7 @@ class FingerprintAuthenticationClient extends AuthenticationClient<IBiometricsFi @Override public void onPointerUp() { mIsPointerDown = false; + mState = STATE_STARTED_PAUSED; UdfpsHelper.onFingerUp(getFreshDaemon()); if (getListener() != null) { try { diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java index a8bf0c751e87..1fe41234849f 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java @@ -372,6 +372,11 @@ public class BiometricSchedulerTest { protected void startHalOperation() { } + + @Override + protected void handleLifecycleAfterAuth(boolean authenticated) { + + } } private static class TestAuthenticationClient extends AuthenticationClient<Object> { @@ -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 9c42f4558450..fb05825a122b 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_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() { + 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<CoexCoordinator.SuccessfulAuth> 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)); } } |