From 15e1d3f7e73dad463c19c7a9275c2ec0ec088326 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Mon, 29 Mar 2021 23:06:03 -0700 Subject: [PATCH 1/3] Update removeAll path 1) Make the resulting face/fingerprint can be @Nullable. Not all removal callers care about the template that was removed, just that they were all removed. 2) FingerprintService/FaceService should wait for all providers to finish removing before notifying the upper layer of completion Fixes: 183758063 Test: atest CtsBiometricsTestCases, notice testLockoutResetRequestedAfterCredentialUnlock finishes 10 seconds quicker now (latch in LSS doesn't time out anymore) Change-Id: I87c50ccf5524fd5440c9a6c2c7a0fd4cc61e684d --- .../android/hardware/face/FaceManager.java | 6 +- .../hardware/face/FaceServiceReceiver.java | 97 +++++++++++++++++++ .../fingerprint/FingerprintManager.java | 2 +- .../FingerprintServiceReceiver.java | 77 +++++++++++++++ .../biometrics/sensors/face/FaceService.java | 22 ++++- .../fingerprint/FingerprintService.java | 24 ++++- 6 files changed, 219 insertions(+), 9 deletions(-) create mode 100644 core/java/android/hardware/face/FaceServiceReceiver.java create mode 100644 core/java/android/hardware/fingerprint/FingerprintServiceReceiver.java diff --git a/core/java/android/hardware/face/FaceManager.java b/core/java/android/hardware/face/FaceManager.java index 8dc8d5b60943b..13e2700b3f54d 100644 --- a/core/java/android/hardware/face/FaceManager.java +++ b/core/java/android/hardware/face/FaceManager.java @@ -1059,7 +1059,7 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan * * @param face The face template that was removed. */ - public void onRemovalSucceeded(Face face, int remaining) { + public void onRemovalSucceeded(@Nullable Face face, int remaining) { } } @@ -1258,10 +1258,6 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan if (mRemovalCallback == null) { return; } - if (face == null) { - Slog.e(TAG, "Received MSG_REMOVED, but face is null"); - return; - } mRemovalCallback.onRemovalSucceeded(face, remaining); } diff --git a/core/java/android/hardware/face/FaceServiceReceiver.java b/core/java/android/hardware/face/FaceServiceReceiver.java new file mode 100644 index 0000000000000..f0f975dcea570 --- /dev/null +++ b/core/java/android/hardware/face/FaceServiceReceiver.java @@ -0,0 +1,97 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware.face; + +import android.os.RemoteException; + +/** + * Provides default methods for callers who only need a subset of the functionality. + * @hide + */ +public class FaceServiceReceiver extends IFaceServiceReceiver.Stub { + @Override + public void onEnrollResult(Face face, int remaining) throws RemoteException { + + } + + @Override + public void onAcquired(int acquiredInfo, int vendorCode) throws RemoteException { + + } + + @Override + public void onAuthenticationSucceeded(Face face, int userId, boolean isStrongBiometric) + throws RemoteException { + + } + + @Override + public void onFaceDetected(int sensorId, int userId, boolean isStrongBiometric) + throws RemoteException { + + } + + @Override + public void onAuthenticationFailed() throws RemoteException { + + } + + @Override + public void onError(int error, int vendorCode) throws RemoteException { + + } + + @Override + public void onRemoved(Face face, int remaining) throws RemoteException { + + } + + @Override + public void onFeatureSet(boolean success, int feature) throws RemoteException { + + } + + @Override + public void onFeatureGet(boolean success, int feature, boolean value) throws RemoteException { + + } + + @Override + public void onChallengeGenerated(int sensorId, long challenge) throws RemoteException { + + } + + @Override + public void onChallengeInterrupted(int sensorId) throws RemoteException { + + } + + @Override + public void onChallengeInterruptFinished(int sensorId) throws RemoteException { + + } + + @Override + public void onAuthenticationFrame(FaceAuthenticationFrame frame) throws RemoteException { + + } + + @Override + public void onEnrollmentFrame(FaceEnrollFrame frame) throws RemoteException { + + } +} diff --git a/core/java/android/hardware/fingerprint/FingerprintManager.java b/core/java/android/hardware/fingerprint/FingerprintManager.java index 1c33b26dfa18a..7b6e1299e8e1e 100644 --- a/core/java/android/hardware/fingerprint/FingerprintManager.java +++ b/core/java/android/hardware/fingerprint/FingerprintManager.java @@ -452,7 +452,7 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing * {@link #remove} is called on a group, this should be the number of remaining * fingerprints in the group, and 0 after the last fingerprint is removed. */ - public void onRemovalSucceeded(Fingerprint fp, int remaining) { } + public void onRemovalSucceeded(@Nullable Fingerprint fp, int remaining) { } } /** diff --git a/core/java/android/hardware/fingerprint/FingerprintServiceReceiver.java b/core/java/android/hardware/fingerprint/FingerprintServiceReceiver.java new file mode 100644 index 0000000000000..798e87beb52a7 --- /dev/null +++ b/core/java/android/hardware/fingerprint/FingerprintServiceReceiver.java @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware.fingerprint; + +import android.os.RemoteException; + +/** + * Provides default methods for callers who only need a subset of the functionality. + * @hide + */ +public class FingerprintServiceReceiver extends IFingerprintServiceReceiver.Stub { + @Override + public void onEnrollResult(Fingerprint fp, int remaining) throws RemoteException { + + } + + @Override + public void onAcquired(int acquiredInfo, int vendorCode) throws RemoteException { + + } + + @Override + public void onAuthenticationSucceeded(Fingerprint fp, int userId, boolean isStrongBiometric) + throws RemoteException { + + } + + @Override + public void onFingerprintDetected(int sensorId, int userId, boolean isStrongBiometric) + throws RemoteException { + + } + + @Override + public void onAuthenticationFailed() throws RemoteException { + + } + + @Override + public void onError(int error, int vendorCode) throws RemoteException { + + } + + @Override + public void onRemoved(Fingerprint fp, int remaining) throws RemoteException { + + } + + @Override + public void onChallengeGenerated(int sensorId, long challenge) throws RemoteException { + + } + + @Override + public void onUdfpsPointerDown(int sensorId) throws RemoteException { + + } + + @Override + public void onUdfpsPointerUp(int sensorId) throws RemoteException { + + } +} diff --git a/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java b/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java index d10fd4f7fa251..a1e6973f5cdc2 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java @@ -36,6 +36,7 @@ import android.hardware.biometrics.face.IFace; import android.hardware.biometrics.face.SensorProps; import android.hardware.face.Face; import android.hardware.face.FaceSensorPropertiesInternal; +import android.hardware.face.FaceServiceReceiver; import android.hardware.face.IFaceService; import android.hardware.face.IFaceServiceReceiver; import android.os.Binder; @@ -394,10 +395,29 @@ public class FaceService extends SystemService implements BiometricServiceCallba final IFaceServiceReceiver receiver, final String opPackageName) { Utils.checkPermission(getContext(), USE_BIOMETRIC_INTERNAL); + final FaceServiceReceiver internalReceiver = new FaceServiceReceiver() { + int sensorsFinishedRemoving = 0; + final int numSensors = getSensorPropertiesInternal( + getContext().getOpPackageName()).size(); + @Override + public void onRemoved(Face face, int remaining) throws RemoteException { + if (remaining == 0) { + sensorsFinishedRemoving++; + Slog.d(TAG, "sensorsFinishedRemoving: " + sensorsFinishedRemoving + + ", numSensors: " + numSensors); + if (sensorsFinishedRemoving == numSensors) { + receiver.onRemoved(null, 0 /* remaining */); + } + } + } + }; + + // This effectively iterates through all sensors, but has to do so by finding all + // sensors under each provider. for (ServiceProvider provider : mServiceProviders) { List props = provider.getSensorProperties(); for (FaceSensorPropertiesInternal prop : props) { - provider.scheduleRemoveAll(prop.sensorId, token, userId, receiver, + provider.scheduleRemoveAll(prop.sensorId, token, userId, internalReceiver, opPackageName); } } diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java index e4397fd158e11..d1776e339af3f 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java @@ -49,6 +49,7 @@ import android.hardware.biometrics.fingerprint.SensorProps; import android.hardware.fingerprint.Fingerprint; import android.hardware.fingerprint.FingerprintManager; import android.hardware.fingerprint.FingerprintSensorPropertiesInternal; +import android.hardware.fingerprint.FingerprintServiceReceiver; import android.hardware.fingerprint.IFingerprintClientActiveCallback; import android.hardware.fingerprint.IFingerprintService; import android.hardware.fingerprint.IFingerprintServiceReceiver; @@ -505,12 +506,31 @@ public class FingerprintService extends SystemService implements BiometricServic @Override // Binder call public void removeAll(final IBinder token, final int userId, final IFingerprintServiceReceiver receiver, final String opPackageName) { - Utils.checkPermission(getContext(), MANAGE_FINGERPRINT); + Utils.checkPermission(getContext(), USE_BIOMETRIC_INTERNAL); + final FingerprintServiceReceiver internalReceiver = new FingerprintServiceReceiver() { + int sensorsFinishedRemoving = 0; + final int numSensors = getSensorPropertiesInternal( + getContext().getOpPackageName()).size(); + @Override + public void onRemoved(Fingerprint fp, int remaining) throws RemoteException { + if (remaining == 0) { + sensorsFinishedRemoving++; + Slog.d(TAG, "sensorsFinishedRemoving: " + sensorsFinishedRemoving + + ", numSensors: " + numSensors); + if (sensorsFinishedRemoving == numSensors) { + receiver.onRemoved(null, 0 /* remaining */); + } + } + } + }; + + // This effectively iterates through all sensors, but has to do so by finding all + // sensors under each provider. for (ServiceProvider provider : mServiceProviders) { List props = provider.getSensorProperties(); for (FingerprintSensorPropertiesInternal prop : props) { - provider.scheduleRemoveAll(prop.sensorId, token, receiver, userId, + provider.scheduleRemoveAll(prop.sensorId, token, internalReceiver, userId, opPackageName); } } From be38fec7c4f1cafc805b7392c3c951f8865a73c9 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Mon, 29 Mar 2021 23:37:51 -0700 Subject: [PATCH 2/3] Make FingerprintService/FaceService less chatty Bug: 183755255 Test: Builds Change-Id: I2717748a845bb67d22203138bfcaa0003579209e --- .../server/biometrics/sensors/face/FaceService.java | 7 +------ .../biometrics/sensors/fingerprint/FingerprintService.java | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java b/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java index a1e6973f5cdc2..34a86d3e63985 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/FaceService.java @@ -167,12 +167,7 @@ public class FaceService extends SystemService implements BiometricServiceCallba String opPackageName) { Utils.checkPermission(getContext(), MANAGE_BIOMETRIC); - final List properties = - FaceService.this.getSensorProperties(); - - Slog.d(TAG, "Retrieved sensor properties for: " + opPackageName - + ", sensors: " + properties.size()); - return properties; + return FaceService.this.getSensorProperties(); } @Override // Binder call diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java index d1776e339af3f..cbef6609873eb 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintService.java @@ -146,12 +146,7 @@ public class FingerprintService extends SystemService implements BiometricServic Utils.checkPermission(getContext(), TEST_BIOMETRIC); } - final List properties = - FingerprintService.this.getSensorProperties(); - - Slog.d(TAG, "Retrieved sensor properties for: " + opPackageName - + ", sensors: " + properties.size()); - return properties; + return FingerprintService.this.getSensorProperties(); } @Override From 07ac5fc5fa2bcef9e072f61124039c3d035df156 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Tue, 30 Mar 2021 00:05:53 -0700 Subject: [PATCH 3/3] Update removeAll test path 1) for the old HIDL interfaces, remove(0) means to remove all enrollments. Thus, have the test HAL pass back the fake enrollmentIds instead of just "0" 2) RemovalClient never receives null identifiers. Update it to be @NonNull and remove the unnecessary null check Test: atest CtsBiometricsTestCases Bug: 183755255 Change-Id: I2cffc4afb77c009ef75ad6ebae358e7779917744 --- .../biometrics/sensors/RemovalClient.java | 8 ++--- .../biometrics/sensors/face/hidl/Face10.java | 2 +- .../biometrics/sensors/face/hidl/TestHal.java | 36 +++++++++++++++++-- .../fingerprint/hidl/Fingerprint21.java | 2 +- .../sensors/fingerprint/hidl/TestHal.java | 29 ++++++++++++++- 5 files changed, 66 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/sensors/RemovalClient.java b/services/core/java/com/android/server/biometrics/sensors/RemovalClient.java index 8197edc97a053..383efce5bb2a4 100644 --- a/services/core/java/com/android/server/biometrics/sensors/RemovalClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/RemovalClient.java @@ -65,12 +65,10 @@ public abstract class RemovalClient(Arrays.asList(faceId)), - 0 /* userId */); + if (faceId == 0) { + // For this HAL interface, remove(0) means to remove all enrollments. + final List faces = FaceUtils.getInstance(mSensorId) + .getBiometricsForUser(mContext, mUserId); + final ArrayList faceIds = new ArrayList<>(); + for (Face face : faces) { + faceIds.add(face.getBiometricId()); + } + mCallback.onRemoved(0 /* deviceId */, faceIds, mUserId); + } else { + mCallback.onRemoved(0 /* deviceId */, + new ArrayList<>(Collections.singletonList(faceId)), + mUserId); + } } return 0; } 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 f2992cccbe099..e149ca9428dde 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 @@ -402,7 +402,7 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider @VisibleForTesting synchronized IBiometricsFingerprint getDaemon() { if (mTestHalEnabled) { - final TestHal testHal = new TestHal(); + final TestHal testHal = new TestHal(mContext, mSensorId); testHal.setNotify(mHalResultController); return testHal; } diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/TestHal.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/TestHal.java index 129f6a61df04c..986e92e244f0e 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/TestHal.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/TestHal.java @@ -16,22 +16,38 @@ package com.android.server.biometrics.sensors.fingerprint.hidl; +import android.annotation.NonNull; import android.annotation.Nullable; +import android.content.Context; import android.hardware.biometrics.fingerprint.V2_1.FingerprintError; import android.hardware.biometrics.fingerprint.V2_1.IBiometricsFingerprintClientCallback; import android.hardware.biometrics.fingerprint.V2_3.IBiometricsFingerprint; +import android.hardware.fingerprint.Fingerprint; import android.os.RemoteException; import android.util.Slog; +import com.android.server.biometrics.sensors.fingerprint.FingerprintUtils; + +import java.util.List; + /** * Test HAL that provides only provides no-ops. */ public class TestHal extends IBiometricsFingerprint.Stub { private static final String TAG = "fingerprint.hidl.TestHal"; + @NonNull + private final Context mContext; + private final int mSensorId; + @Nullable private IBiometricsFingerprintClientCallback mCallback; + TestHal(@NonNull Context context, int sensorId) { + mContext = context; + mSensorId = sensorId; + } + @Override public boolean isUdfps(int sensorId) { return false; @@ -96,7 +112,18 @@ public class TestHal extends IBiometricsFingerprint.Stub { public int remove(int gid, int fid) throws RemoteException { Slog.w(TAG, "Remove"); if (mCallback != null) { - mCallback.onRemoved(0 /* deviceId */, fid, gid, 0 /* remaining */); + if (fid == 0) { + // For this HAL interface, remove(0) means to remove all enrollments. + final List fingerprints = FingerprintUtils.getInstance(mSensorId) + .getBiometricsForUser(mContext, gid); + for (int i = 0; i < fingerprints.size(); i++) { + final Fingerprint fp = fingerprints.get(i); + mCallback.onRemoved(0 /* deviceId */, fp.getBiometricId(), gid, + fingerprints.size() - i - 1); + } + } else { + mCallback.onRemoved(0 /* deviceId */, fid, gid, 0 /* remaining */); + } } return 0; }