From e44a86b2edebe3b57c5104514dbbcb9de2856d21 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Wed, 5 Aug 2020 21:56:31 -0700 Subject: [PATCH] 2/n: Keep track of IBiometricsFace@1.0 challenge interruptions IBiometricsFace@1.0 only supports a single in-flight challenge. Currently, challenge owners paths are designed to never conflict lifecycle-wise. However, to decouple resetLockout (which relies on challenge) from the critical user unlock path, we need to keep track of challenge interruptions, and notify clients when an interruption starts/finishes. This change only adds the infrastructure for tracking challenge interruptions. A following change will make use of this. Bug: 145978626 Test: Builds Change-Id: I708312cafad29614e3708b5c606ae8a90704ac66 --- .../android/hardware/face/FaceManager.java | 49 +++++++++++++++ .../hardware/face/IFaceServiceReceiver.aidl | 2 + .../ClientMonitorCallbackConverter.java | 15 ++++- .../biometrics/sensors/face/Face10.java | 61 ++++++++++++++++++- .../face/FaceGenerateChallengeClient.java | 16 ++++- 5 files changed, 137 insertions(+), 6 deletions(-) diff --git a/core/java/android/hardware/face/FaceManager.java b/core/java/android/hardware/face/FaceManager.java index 19cb13c7e1148..4f949cfc62a57 100644 --- a/core/java/android/hardware/face/FaceManager.java +++ b/core/java/android/hardware/face/FaceManager.java @@ -72,6 +72,8 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan private static final int MSG_SET_FEATURE_COMPLETED = 107; private static final int MSG_CHALLENGE_GENERATED = 108; private static final int MSG_FACE_DETECTED = 109; + private static final int MSG_CHALLENGE_INTERRUPTED = 110; + private static final int MSG_CHALLENGE_INTERRUPT_FINISHED = 111; private final IFaceService mService; private final Context mContext; @@ -150,6 +152,16 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan mHandler.obtainMessage(MSG_CHALLENGE_GENERATED, challenge).sendToTarget(); } } + + @Override + public void onChallengeInterrupted(int sensorId) { + mHandler.obtainMessage(MSG_CHALLENGE_INTERRUPTED, sensorId).sendToTarget(); + } + + @Override + public void onChallengeInterruptFinished(int sensorId) { + mHandler.obtainMessage(MSG_CHALLENGE_INTERRUPT_FINISHED, sensorId).sendToTarget(); + } }; /** @@ -1071,10 +1083,25 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } /** + * Callback structure provided to {@link #generateChallenge(GenerateChallengeCallback)}. * @hide */ public interface GenerateChallengeCallback { + /** + * Invoked when a challenge has been generated. + */ void onGenerateChallengeResult(long challenge); + + /** + * Invoked if the challenge has not been revoked and a subsequent caller/owner invokes + * {@link #generateChallenge(GenerateChallengeCallback)}, but + */ + default void onChallengeInterrupted(int sensorId) {} + + /** + * Invoked when the interrupting client has finished (e.g. revoked its challenge). + */ + default void onChallengeInterruptFinished(int sensorId) {} } private abstract static class InternalGenerateChallengeCallback @@ -1157,6 +1184,12 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan sendFaceDetected(msg.arg1 /* sensorId */, msg.arg2 /* userId */, (boolean) msg.obj /* isStrongBiometric */); break; + case MSG_CHALLENGE_INTERRUPTED: + sendChallengeInterrupted((int) msg.obj /* sensorId */); + break; + case MSG_CHALLENGE_INTERRUPT_FINISHED: + sendChallengeInterruptFinished((int) msg.obj /* sensorId */); + break; default: Slog.w(TAG, "Unknown message: " + msg.what); } @@ -1193,6 +1226,22 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan mFaceDetectionCallback.onFaceDetected(sensorId, userId, isStrongBiometric); } + private void sendChallengeInterrupted(int sensorId) { + if (mGenerateChallengeCallback == null) { + Slog.e(TAG, "sendChallengeInterrupted, callback null"); + return; + } + mGenerateChallengeCallback.onChallengeInterrupted(sensorId); + } + + private void sendChallengeInterruptFinished(int sensorId) { + if (mGenerateChallengeCallback == null) { + Slog.e(TAG, "sendChallengeInterruptFinished, callback null"); + return; + } + mGenerateChallengeCallback.onChallengeInterruptFinished(sensorId); + } + private void sendRemovedResult(Face face, int remaining) { if (mRemovalCallback == null) { return; diff --git a/core/java/android/hardware/face/IFaceServiceReceiver.aidl b/core/java/android/hardware/face/IFaceServiceReceiver.aidl index 2600b7def03a6..108f4f69bdd8c 100644 --- a/core/java/android/hardware/face/IFaceServiceReceiver.aidl +++ b/core/java/android/hardware/face/IFaceServiceReceiver.aidl @@ -32,4 +32,6 @@ oneway interface IFaceServiceReceiver { void onFeatureSet(boolean success, int feature); void onFeatureGet(boolean success, int feature, boolean value); void onChallengeGenerated(long challenge); + void onChallengeInterrupted(int sensorId); + void onChallengeInterruptFinished(int sensorId); } diff --git a/services/core/java/com/android/server/biometrics/sensors/ClientMonitorCallbackConverter.java b/services/core/java/com/android/server/biometrics/sensors/ClientMonitorCallbackConverter.java index 3cef6667b6446..2c60d096bd136 100644 --- a/services/core/java/com/android/server/biometrics/sensors/ClientMonitorCallbackConverter.java +++ b/services/core/java/com/android/server/biometrics/sensors/ClientMonitorCallbackConverter.java @@ -142,10 +142,21 @@ public final class ClientMonitorCallbackConverter { } } - public void onFeatureGet(boolean success, int feature, boolean value) - throws RemoteException { + public void onFeatureGet(boolean success, int feature, boolean value) throws RemoteException { if (mFaceServiceReceiver != null) { mFaceServiceReceiver.onFeatureGet(success, feature, value); } } + + public void onChallengeInterrupted(int sensorId) throws RemoteException { + if (mFaceServiceReceiver != null) { + mFaceServiceReceiver.onChallengeInterrupted(sensorId); + } + } + + public void onChallengeInterruptFinished(int sensorId) throws RemoteException { + if (mFaceServiceReceiver != null) { + mFaceServiceReceiver.onChallengeInterruptFinished(sensorId); + } + } } diff --git a/services/core/java/com/android/server/biometrics/sensors/face/Face10.java b/services/core/java/com/android/server/biometrics/sensors/face/Face10.java index 8a746e2de4db8..a4956cb565278 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/Face10.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/Face10.java @@ -98,6 +98,11 @@ class Face10 implements IHwBinder.DeathRecipient { @Nullable private IBiometricsFace mDaemon; private int mCurrentUserId = UserHandle.USER_NULL; + // If a challenge is generated, keep track of its owner. Since IBiometricsFace@1.0 only + // supports a single in-flight challenge, we must notify the interrupted owner that its + // challenge is no longer valid. The interrupted owner will be notified when the interrupter + // has finished. + @Nullable private FaceGenerateChallengeClient mCurrentChallengeOwner; private final UserSwitchObserver mUserSwitchObserver = new SynchronousUserSwitchObserver() { @Override @@ -462,18 +467,68 @@ class Face10 implements IHwBinder.DeathRecipient { void scheduleGenerateChallenge(@NonNull IBinder token, @NonNull IFaceServiceReceiver receiver, @NonNull String opPackageName) { mHandler.post(() -> { + if (mCurrentChallengeOwner != null) { + Slog.w(TAG, "Current challenge owner: " + mCurrentChallengeOwner + + ", interrupted by: " + opPackageName); + try { + mCurrentChallengeOwner.getListener().onChallengeInterrupted(mSensorId); + } catch (RemoteException e) { + Slog.e(TAG, "Unable to notify challenge interrupted", e); + } + } + final FaceGenerateChallengeClient client = new FaceGenerateChallengeClient(mContext, mLazyDaemon, token, new ClientMonitorCallbackConverter(receiver), opPackageName, - mSensorId); - mScheduler.scheduleClientMonitor(client); + mSensorId, mCurrentChallengeOwner); + mScheduler.scheduleClientMonitor(client, new ClientMonitor.Callback() { + @Override + public void onClientStarted(@NonNull ClientMonitor clientMonitor) { + if (client != clientMonitor) { + Slog.e(TAG, "scheduleGenerateChallenge, mismatched client." + + " Expecting: " + client + ", received: " + clientMonitor); + return; + } + Slog.d(TAG, "Current challenge owner: " + client); + mCurrentChallengeOwner = client; + } + }); }); } void scheduleRevokeChallenge(@NonNull IBinder token, @NonNull String owner) { mHandler.post(() -> { + if (!mCurrentChallengeOwner.getOwnerString().contentEquals(owner)) { + Slog.e(TAG, "Package: " + owner + " attempting to revoke challenge owned by: " + + mCurrentChallengeOwner.getOwnerString()); + return; + } + final FaceRevokeChallengeClient client = new FaceRevokeChallengeClient(mContext, mLazyDaemon, token, owner, mSensorId); - mScheduler.scheduleClientMonitor(client); + mScheduler.scheduleClientMonitor(client, new ClientMonitor.Callback() { + @Override + public void onClientFinished(@NonNull ClientMonitor clientMonitor, + boolean success) { + if (client != clientMonitor) { + Slog.e(TAG, "scheduleRevokeChallenge, mismatched client." + + "Expecting: " + client + ", received: " + clientMonitor); + return; + } + + final FaceGenerateChallengeClient previousChallengeOwner = + mCurrentChallengeOwner.getInterruptedClient(); + mCurrentChallengeOwner = null; + Slog.d(TAG, "Previous challenge owner: " + previousChallengeOwner); + if (previousChallengeOwner != null) { + try { + previousChallengeOwner.getListener() + .onChallengeInterruptFinished(mSensorId); + } catch (RemoteException e) { + Slog.e(TAG, "Unable to notify interrupt finished", e); + } + } + } + }); }); } diff --git a/services/core/java/com/android/server/biometrics/sensors/face/FaceGenerateChallengeClient.java b/services/core/java/com/android/server/biometrics/sensors/face/FaceGenerateChallengeClient.java index 67f2712d0b9d9..ba401f255e43b 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/FaceGenerateChallengeClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/FaceGenerateChallengeClient.java @@ -17,12 +17,14 @@ package com.android.server.biometrics.sensors.face; import android.annotation.NonNull; +import android.annotation.Nullable; import android.content.Context; import android.hardware.biometrics.face.V1_0.IBiometricsFace; import android.os.IBinder; import android.os.RemoteException; import android.util.Slog; +import com.android.server.biometrics.sensors.ClientMonitor; import com.android.server.biometrics.sensors.ClientMonitorCallbackConverter; import com.android.server.biometrics.sensors.GenerateChallengeClient; @@ -36,10 +38,22 @@ public class FaceGenerateChallengeClient extends GenerateChallengeClient lazyDaemon, @NonNull IBinder token, - @NonNull ClientMonitorCallbackConverter listener, @NonNull String owner, int sensorId) { + @NonNull ClientMonitorCallbackConverter listener, @NonNull String owner, int sensorId, + @Nullable FaceGenerateChallengeClient interruptedClient) { super(context, lazyDaemon, token, listener, owner, sensorId); + mInterruptedClient = interruptedClient; + } + + @Nullable + public FaceGenerateChallengeClient getInterruptedClient() { + return mInterruptedClient; } @Override