From d1bb072d968a469b040a040f1d049486888bf789 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Fri, 17 Jul 2020 13:51:04 -0700 Subject: [PATCH] 3/n: Move generateChallenge/resetLockout/revokeChallenge off critical path 1) Introduces BiometricDeferredQueue in LockSettings package to move slow operations off of the critical unlock path. 2) Changes generateChallenge to require sensorId. Callers to generateChallengeBlocking are currently not affected. Their path will need to be updated in the future. 3) Adds resetLockoutRequiresHardwareAuthToken for fingerprint sensor properties, since IBiometricsFingerprint@2.1 and its derivatives do not require the HAT yet Fixes: 145978626 Test: atest com.android.server.biometrics Test: Able to enroll after entering password Test: Reset lockout on face device with single-profile-per-user Test: Reset lockout on face device with managed profile + unified credential Test: Reset lockout on face device with managed profile and separate credential (both owner and managed profile) Test: Reset lockout for secondary user Change-Id: Id4d7c39274a52ef61709161b6f24ec4f5d76720e --- .../hardware/biometrics/BiometricManager.java | 20 -- .../hardware/biometrics/IAuthService.aidl | 3 - .../biometrics/IBiometricAuthenticator.aidl | 3 - .../biometrics/IBiometricService.aidl | 3 - .../android/hardware/face/FaceManager.java | 115 ++++---- .../android/hardware/face/IFaceService.aidl | 6 +- .../hardware/face/IFaceServiceReceiver.aidl | 2 +- .../fingerprint/FingerprintManager.java | 65 +++-- .../FingerprintSensorProperties.java | 9 +- .../fingerprint/IFingerprintService.aidl | 4 +- .../IFingerprintServiceReceiver.aidl | 2 +- .../server/biometrics/AuthService.java | 11 - .../server/biometrics/BiometricService.java | 13 - .../ClientMonitorCallbackConverter.java | 6 +- .../sensors/GenerateChallengeClient.java | 4 +- .../biometrics/sensors/face/Face10.java | 19 +- .../sensors/face/FaceAuthenticator.java | 5 - .../biometrics/sensors/face/FaceService.java | 30 ++- .../sensors/fingerprint/Fingerprint21.java | 7 +- .../fingerprint/Fingerprint21UdfpsMock.java | 4 +- .../fingerprint/FingerprintAuthenticator.java | 5 - .../fingerprint/FingerprintService.java | 24 +- .../sensors/iris/IrisAuthenticator.java | 4 - .../locksettings/BiometricDeferredQueue.java | 252 ++++++++++++++++++ .../locksettings/LockSettingsService.java | 175 +++--------- .../server/biometrics/AuthServiceTest.java | 15 -- 26 files changed, 492 insertions(+), 314 deletions(-) create mode 100644 services/core/java/com/android/server/locksettings/BiometricDeferredQueue.java diff --git a/core/java/android/hardware/biometrics/BiometricManager.java b/core/java/android/hardware/biometrics/BiometricManager.java index a605f5d68f062..f86eb9082e01d 100644 --- a/core/java/android/hardware/biometrics/BiometricManager.java +++ b/core/java/android/hardware/biometrics/BiometricManager.java @@ -299,26 +299,6 @@ public class BiometricManager { } } - /** - * Reset the lockout when user authenticates with strong auth (e.g. PIN, pattern or password) - * - * @param userId this operation takes effect for. - * @param hardwareAuthToken an opaque token returned by password confirmation. - * @hide - */ - @RequiresPermission(USE_BIOMETRIC_INTERNAL) - public void resetLockout(int userId, byte[] hardwareAuthToken) { - if (mService != null) { - try { - mService.resetLockout(userId, hardwareAuthToken); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } - } else { - Slog.w(TAG, "resetLockout(): Service not connected"); - } - } - /** * Get a list of AuthenticatorIDs for biometric authenticators which have 1) enrolled templates, * and 2) meet the requirements for integrating with Keystore. The AuthenticatorIDs are known diff --git a/core/java/android/hardware/biometrics/IAuthService.aidl b/core/java/android/hardware/biometrics/IAuthService.aidl index c1d0ad44850dc..dd6aa736a1a35 100644 --- a/core/java/android/hardware/biometrics/IAuthService.aidl +++ b/core/java/android/hardware/biometrics/IAuthService.aidl @@ -46,9 +46,6 @@ interface IAuthService { // Register callback for when keyguard biometric eligibility changes. void registerEnabledOnKeyguardCallback(IBiometricEnabledOnKeyguardCallback callback); - // Reset the lockout when user authenticates with strong auth (e.g. PIN, pattern or password) - void resetLockout(int userId, in byte [] hardwareAuthToken); - // Get a list of AuthenticatorIDs for authenticators which have enrolled templates and meet // the requirements for integrating with Keystore. The AuthenticatorID are known in Keystore // land as SIDs, and are used during key generation. diff --git a/core/java/android/hardware/biometrics/IBiometricAuthenticator.aidl b/core/java/android/hardware/biometrics/IBiometricAuthenticator.aidl index 8eb22dabbf3c9..5e6fe6885f9de 100644 --- a/core/java/android/hardware/biometrics/IBiometricAuthenticator.aidl +++ b/core/java/android/hardware/biometrics/IBiometricAuthenticator.aidl @@ -53,9 +53,6 @@ interface IBiometricAuthenticator { // Return the LockoutTracker status for the specified user int getLockoutModeForUser(int userId); - // Reset the lockout when user authenticates with strong auth (e.g. PIN, pattern or password) - void resetLockout(int userId, in byte [] hardwareAuthToken); - // Gets the authenticator ID representing the current set of enrolled templates long getAuthenticatorId(int callingUserId); } diff --git a/core/java/android/hardware/biometrics/IBiometricService.aidl b/core/java/android/hardware/biometrics/IBiometricService.aidl index a5b3abb30cbbd..005ed324da775 100644 --- a/core/java/android/hardware/biometrics/IBiometricService.aidl +++ b/core/java/android/hardware/biometrics/IBiometricService.aidl @@ -56,9 +56,6 @@ interface IBiometricService { // Client lifecycle is still managed in Service. void onReadyForAuthentication(int cookie); - // Reset the lockout when user authenticates with strong auth (e.g. PIN, pattern or password) - void resetLockout(int userId, in byte [] hardwareAuthToken); - // Get a list of AuthenticatorIDs for authenticators which have enrolled templates and meet // the requirements for integrating with Keystore. The AuthenticatorID are known in Keystore // land as SIDs, and are used during key generation. diff --git a/core/java/android/hardware/face/FaceManager.java b/core/java/android/hardware/face/FaceManager.java index 4f949cfc62a57..1b114d3528a2e 100644 --- a/core/java/android/hardware/face/FaceManager.java +++ b/core/java/android/hardware/face/FaceManager.java @@ -143,14 +143,9 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } @Override - public void onChallengeGenerated(long challenge) { - if (mGenerateChallengeCallback instanceof InternalGenerateChallengeCallback) { - // Perform this on system_server thread, since the application's thread is - // blocked waiting for the result - mGenerateChallengeCallback.onGenerateChallengeResult(challenge); - } else { - mHandler.obtainMessage(MSG_CHALLENGE_GENERATED, challenge).sendToTarget(); - } + public void onChallengeGenerated(int sensorId, long challenge) { + mHandler.obtainMessage(MSG_CHALLENGE_GENERATED, sensorId, 0, challenge) + .sendToTarget(); } @Override @@ -416,35 +411,6 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } } - /** - * Same as {@link #generateChallenge(GenerateChallengeCallback)}, except blocks until the - * TEE/hardware operation is complete. - * @return challenge generated in the TEE/hardware - * @hide - */ - @RequiresPermission(MANAGE_BIOMETRIC) - public long generateChallengeBlocking() { - final AtomicReference result = new AtomicReference<>(); - final CountDownLatch latch = new CountDownLatch(1); - final GenerateChallengeCallback callback = new InternalGenerateChallengeCallback() { - @Override - public void onGenerateChallengeResult(long challenge) { - result.set(challenge); - latch.countDown(); - } - }; - - generateChallenge(callback); - - try { - latch.await(1, TimeUnit.SECONDS); - } catch (InterruptedException e) { - Slog.e(TAG, "Interrupted while generatingChallenge", e); - e.printStackTrace(); - } - return result.get(); - } - /** * Generates a unique random challenge in the TEE. A typical use case is to have it wrapped in a * HardwareAuthenticationToken, minted by Gatekeeper upon PIN/Pattern/Password verification. @@ -458,11 +424,12 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan * @hide */ @RequiresPermission(MANAGE_BIOMETRIC) - public void generateChallenge(GenerateChallengeCallback callback) { + public void generateChallenge(int sensorId, GenerateChallengeCallback callback) { if (mService != null) { try { mGenerateChallengeCallback = callback; - mService.generateChallenge(mToken, mServiceReceiver, mContext.getOpPackageName()); + mService.generateChallenge(mToken, sensorId, mServiceReceiver, + mContext.getOpPackageName()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -470,15 +437,66 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } /** - * Invalidates the current auth token. + * Same as {@link #generateChallenge(int, GenerateChallengeCallback)}, but assumes the first + * enumerated sensor. + * @hide + */ + @RequiresPermission(MANAGE_BIOMETRIC) + public void generateChallenge(GenerateChallengeCallback callback) { + final List faceSensorProperties = getSensorProperties(); + if (faceSensorProperties.isEmpty()) { + Slog.e(TAG, "No sensors"); + return; + } + + final int sensorId = faceSensorProperties.get(0).sensorId; + generateChallenge(sensorId, callback); + } + + /** + * Invalidates the current challenge. * * @hide */ @RequiresPermission(MANAGE_BIOMETRIC) public void revokeChallenge() { + final List faceSensorProperties = getSensorProperties(); + if (faceSensorProperties.isEmpty()) { + Slog.e(TAG, "No sensors during revokeChallenge"); + } + revokeChallenge(faceSensorProperties.get(0).sensorId); + } + + /** + * Invalidates the current challenge. + * + * @hide + */ + @RequiresPermission(MANAGE_BIOMETRIC) + public void revokeChallenge(int sensorId) { if (mService != null) { try { - mService.revokeChallenge(mToken, mContext.getOpPackageName()); + mService.revokeChallenge(mToken, sensorId, mContext.getOpPackageName()); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + } + + /** + * Reset the lockout when user authenticates with strong auth (e.g. PIN, pattern or password) + * + * @param sensorId Sensor ID that this operation takes effect for + * @param userId User ID that this operation takes effect for. + * @param hardwareAuthToken An opaque token returned by password confirmation. + * @hide + */ + @RequiresPermission(USE_BIOMETRIC_INTERNAL) + public void resetLockout(int sensorId, int userId, @Nullable byte[] hardwareAuthToken) { + if (mService != null) { + try { + mService.resetLockout(mToken, sensorId, userId, hardwareAuthToken, + mContext.getOpPackageName()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -1083,18 +1101,18 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } /** - * Callback structure provided to {@link #generateChallenge(GenerateChallengeCallback)}. + * Callback structure provided to {@link #generateChallenge(int, GenerateChallengeCallback)}. * @hide */ public interface GenerateChallengeCallback { /** * Invoked when a challenge has been generated. */ - void onGenerateChallengeResult(long challenge); + void onGenerateChallengeResult(int sensorId, long challenge); /** * Invoked if the challenge has not been revoked and a subsequent caller/owner invokes - * {@link #generateChallenge(GenerateChallengeCallback)}, but + * {@link #generateChallenge(int, GenerateChallengeCallback)}, but */ default void onChallengeInterrupted(int sensorId) {} @@ -1104,9 +1122,6 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan default void onChallengeInterruptFinished(int sensorId) {} } - private abstract static class InternalGenerateChallengeCallback - implements GenerateChallengeCallback {} - private class OnEnrollCancelListener implements OnCancelListener { @Override public void onCancel() { @@ -1178,7 +1193,7 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan args.recycle(); break; case MSG_CHALLENGE_GENERATED: - sendChallengeGenerated((long) msg.obj /* challenge */); + sendChallengeGenerated(msg.arg1 /* sensorId */, (long) msg.obj /* challenge */); break; case MSG_FACE_DETECTED: sendFaceDetected(msg.arg1 /* sensorId */, msg.arg2 /* userId */, @@ -1211,11 +1226,11 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan mGetFeatureCallback.onCompleted(success, feature, value); } - private void sendChallengeGenerated(long challenge) { + private void sendChallengeGenerated(int sensorId, long challenge) { if (mGenerateChallengeCallback == null) { return; } - mGenerateChallengeCallback.onGenerateChallengeResult(challenge); + mGenerateChallengeCallback.onGenerateChallengeResult(sensorId, challenge); } private void sendFaceDetected(int sensorId, int userId, boolean isStrongBiometric) { diff --git a/core/java/android/hardware/face/IFaceService.aidl b/core/java/android/hardware/face/IFaceService.aidl index a9097d4013495..437feb13b845f 100644 --- a/core/java/android/hardware/face/IFaceService.aidl +++ b/core/java/android/hardware/face/IFaceService.aidl @@ -83,10 +83,10 @@ interface IFaceService { boolean isHardwareDetected(String opPackageName); // Get a pre-enrollment authentication token - void generateChallenge(IBinder token, IFaceServiceReceiver receiver, String opPackageName); + void generateChallenge(IBinder token, int sensorId, IFaceServiceReceiver receiver, String opPackageName); // Finish an enrollment sequence and invalidate the authentication token - void revokeChallenge(IBinder token, String opPackageName); + void revokeChallenge(IBinder token, int sensorId, String opPackageName); // Determine if a user has at least one enrolled face boolean hasEnrolledFaces(int userId, String opPackageName); @@ -98,7 +98,7 @@ interface IFaceService { long getAuthenticatorId(int callingUserId); // Reset the lockout when user authenticates with strong auth (e.g. PIN, pattern or password) - void resetLockout(int userId, in byte [] hardwareAuthToken); + void resetLockout(IBinder token, int sensorId, int userId, in byte [] hardwareAuthToken, String opPackageName); // Add a callback which gets notified when the face lockout period expired. void addLockoutResetCallback(IBiometricServiceLockoutResetCallback callback, String opPackageName); diff --git a/core/java/android/hardware/face/IFaceServiceReceiver.aidl b/core/java/android/hardware/face/IFaceServiceReceiver.aidl index 108f4f69bdd8c..bd4d3a0b7017f 100644 --- a/core/java/android/hardware/face/IFaceServiceReceiver.aidl +++ b/core/java/android/hardware/face/IFaceServiceReceiver.aidl @@ -31,7 +31,7 @@ oneway interface IFaceServiceReceiver { void onRemoved(in Face face, int remaining); void onFeatureSet(boolean success, int feature); void onFeatureGet(boolean success, int feature, boolean value); - void onChallengeGenerated(long challenge); + void onChallengeGenerated(int sensorId, long challenge); void onChallengeInterrupted(int sensorId); void onChallengeInterruptFinished(int sensorId); } diff --git a/core/java/android/hardware/fingerprint/FingerprintManager.java b/core/java/android/hardware/fingerprint/FingerprintManager.java index 71598eb6394f5..c12bb39c31754 100644 --- a/core/java/android/hardware/fingerprint/FingerprintManager.java +++ b/core/java/android/hardware/fingerprint/FingerprintManager.java @@ -18,6 +18,7 @@ package android.hardware.fingerprint; import static android.Manifest.permission.INTERACT_ACROSS_USERS; import static android.Manifest.permission.MANAGE_FINGERPRINT; +import static android.Manifest.permission.RESET_FINGERPRINT_LOCKOUT; import static android.Manifest.permission.USE_BIOMETRIC; import static android.Manifest.permission.USE_BIOMETRIC_INTERNAL; import static android.Manifest.permission.USE_FINGERPRINT; @@ -378,12 +379,9 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing * @hide */ public interface GenerateChallengeCallback { - void onChallengeGenerated(long challenge); + void onChallengeGenerated(int sensorId, long challenge); } - private abstract static class InternalGenerateChallengeCallback - implements GenerateChallengeCallback {} - /** * Request authentication of a crypto object. This call warms up the fingerprint hardware * and starts scanning for a fingerprint. It terminates when @@ -593,15 +591,33 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing * @hide */ @RequiresPermission(MANAGE_FINGERPRINT) - public void generateChallenge(GenerateChallengeCallback callback) { + public void generateChallenge(int sensorId, GenerateChallengeCallback callback) { if (mService != null) try { mGenerateChallengeCallback = callback; - mService.generateChallenge(mToken, mServiceReceiver, mContext.getOpPackageName()); + mService.generateChallenge(mToken, sensorId, mServiceReceiver, + mContext.getOpPackageName()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } } + /** + * Same as {@link #generateChallenge(int, GenerateChallengeCallback)}, but assumes the first + * enumerated sensor. + * @hide + */ + @RequiresPermission(MANAGE_FINGERPRINT) + public void generateChallenge(GenerateChallengeCallback callback) { + final List fingerprintSensorProperties = getSensorProperties(); + if (fingerprintSensorProperties.isEmpty()) { + Slog.e(TAG, "No sensors"); + return; + } + + final int sensorId = fingerprintSensorProperties.get(0).sensorId; + generateChallenge(sensorId, callback); + } + /** * Finishes enrollment and cancels the current auth token. * @hide @@ -615,6 +631,26 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing } } + /** + * Reset the lockout when user authenticates with strong auth (e.g. PIN, pattern or password) + * + * @param sensorId Sensor ID that this operation takes effect for + * @param userId User ID that this operation takes effect for. + * @param hardwareAuthToken An opaque token returned by password confirmation. + * @hide + */ + @RequiresPermission(RESET_FINGERPRINT_LOCKOUT) + public void resetLockout(int sensorId, int userId, @Nullable byte[] hardwareAuthToken) { + if (mService != null) { + try { + mService.resetLockout(mToken, sensorId, userId, hardwareAuthToken, + mContext.getOpPackageName()); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + } + /** * Remove given fingerprint template from fingerprint hardware and/or protected storage. * @param fp the fingerprint item to remove @@ -901,7 +937,7 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing sendRemovedResult((Fingerprint) msg.obj, msg.arg1 /* remaining */); break; case MSG_CHALLENGE_GENERATED: - sendChallengeGenerated((long) msg.obj /* challenge */); + sendChallengeGenerated(msg.arg1 /* sensorId */, (long) msg.obj /* challenge */); break; case MSG_FINGERPRINT_DETECTED: sendFingerprintDetected(msg.arg1 /* sensorId */, msg.arg2 /* userId */, @@ -989,12 +1025,12 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing } } - private void sendChallengeGenerated(long challenge) { + private void sendChallengeGenerated(int sensorId, long challenge) { if (mGenerateChallengeCallback == null) { Slog.e(TAG, "sendChallengeGenerated, callback null"); return; } - mGenerateChallengeCallback.onChallengeGenerated(challenge); + mGenerateChallengeCallback.onChallengeGenerated(sensorId, challenge); } private void sendFingerprintDetected(int sensorId, int userId, boolean isStrongBiometric) { @@ -1178,14 +1214,9 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing } @Override // binder call - public void onChallengeGenerated(long challenge) { - if (mGenerateChallengeCallback instanceof InternalGenerateChallengeCallback) { - // Perform this on system_server thread, since the application's thread is - // blocked waiting for the result - mGenerateChallengeCallback.onChallengeGenerated(challenge); - } else { - mHandler.obtainMessage(MSG_CHALLENGE_GENERATED, challenge).sendToTarget(); - } + public void onChallengeGenerated(int sensorId, long challenge) { + mHandler.obtainMessage(MSG_CHALLENGE_GENERATED, sensorId, 0, challenge) + .sendToTarget(); } }; diff --git a/core/java/android/hardware/fingerprint/FingerprintSensorProperties.java b/core/java/android/hardware/fingerprint/FingerprintSensorProperties.java index a774121c43f49..b28551c52b9ea 100644 --- a/core/java/android/hardware/fingerprint/FingerprintSensorProperties.java +++ b/core/java/android/hardware/fingerprint/FingerprintSensorProperties.java @@ -45,18 +45,24 @@ public class FingerprintSensorProperties implements Parcelable { public final int sensorId; public final @SensorType int sensorType; + // IBiometricsFingerprint@2.1 does not manage timeout below the HAL, so the Gatekeeper HAT + // cannot be checked + public final boolean resetLockoutRequiresHardwareAuthToken; /** * Initializes SensorProperties with specified values */ - public FingerprintSensorProperties(int sensorId, @SensorType int sensorType) { + public FingerprintSensorProperties(int sensorId, @SensorType int sensorType, + boolean resetLockoutRequiresHardwareAuthToken) { this.sensorId = sensorId; this.sensorType = sensorType; + this.resetLockoutRequiresHardwareAuthToken = resetLockoutRequiresHardwareAuthToken; } protected FingerprintSensorProperties(Parcel in) { sensorId = in.readInt(); sensorType = in.readInt(); + resetLockoutRequiresHardwareAuthToken = in.readBoolean(); } public static final Creator CREATOR = @@ -81,5 +87,6 @@ public class FingerprintSensorProperties implements Parcelable { public void writeToParcel(Parcel dest, int flags) { dest.writeInt(sensorId); dest.writeInt(sensorType); + dest.writeBoolean(resetLockoutRequiresHardwareAuthToken); } } diff --git a/core/java/android/hardware/fingerprint/IFingerprintService.aidl b/core/java/android/hardware/fingerprint/IFingerprintService.aidl index f6069d81934f6..0fae15648e156 100644 --- a/core/java/android/hardware/fingerprint/IFingerprintService.aidl +++ b/core/java/android/hardware/fingerprint/IFingerprintService.aidl @@ -88,7 +88,7 @@ interface IFingerprintService { boolean isHardwareDetected(String opPackageName); // Get a pre-enrollment authentication token - void generateChallenge(IBinder token, IFingerprintServiceReceiver receiver, String opPackageName); + void generateChallenge(IBinder token, int sensorId, IFingerprintServiceReceiver receiver, String opPackageName); // Finish an enrollment sequence and invalidate the authentication token void revokeChallenge(IBinder token, String opPackageName); @@ -103,7 +103,7 @@ interface IFingerprintService { long getAuthenticatorId(int callingUserId); // Reset the timeout when user authenticates with strong auth (e.g. PIN, pattern or password) - void resetLockout(int userId, in byte [] hardwareAuthToken); + void resetLockout(IBinder token, int sensorId, int userId, in byte[] hardwareAuthToken, String opPackageNAame); // Add a callback which gets notified when the fingerprint lockout period expired. void addLockoutResetCallback(IBiometricServiceLockoutResetCallback callback, String opPackageName); diff --git a/core/java/android/hardware/fingerprint/IFingerprintServiceReceiver.aidl b/core/java/android/hardware/fingerprint/IFingerprintServiceReceiver.aidl index ad8fbc087ed0b..095b8e9527ade 100644 --- a/core/java/android/hardware/fingerprint/IFingerprintServiceReceiver.aidl +++ b/core/java/android/hardware/fingerprint/IFingerprintServiceReceiver.aidl @@ -29,5 +29,5 @@ oneway interface IFingerprintServiceReceiver { void onAuthenticationFailed(); void onError(int error, int vendorCode); void onRemoved(in Fingerprint fp, int remaining); - void onChallengeGenerated(long challenge); + void onChallengeGenerated(int sensorId, long challenge); } diff --git a/services/core/java/com/android/server/biometrics/AuthService.java b/services/core/java/com/android/server/biometrics/AuthService.java index a47904c40cc35..54c1790309295 100644 --- a/services/core/java/com/android/server/biometrics/AuthService.java +++ b/services/core/java/com/android/server/biometrics/AuthService.java @@ -258,17 +258,6 @@ public class AuthService extends SystemService { } } - @Override - public void resetLockout(int userId, byte[] hardwareAuthToken) throws RemoteException { - checkInternalPermission(); - final long identity = Binder.clearCallingIdentity(); - try { - mBiometricService.resetLockout(userId, hardwareAuthToken); - } finally { - Binder.restoreCallingIdentity(identity); - } - } - @Override public long[] getAuthenticatorIds() throws RemoteException { // In this method, we're not checking whether the caller is permitted to use face diff --git a/services/core/java/com/android/server/biometrics/BiometricService.java b/services/core/java/com/android/server/biometrics/BiometricService.java index 382bdbb246a1d..3e0a40f9c2885 100644 --- a/services/core/java/com/android/server/biometrics/BiometricService.java +++ b/services/core/java/com/android/server/biometrics/BiometricService.java @@ -637,19 +637,6 @@ public class BiometricService extends SystemService { } } - @Override // Binder call - public void resetLockout(int userId, byte[] hardwareAuthToken) { - checkInternalPermission(); - - try { - for (BiometricSensor sensor : mSensors) { - sensor.impl.resetLockout(userId, hardwareAuthToken); - } - } catch (RemoteException e) { - Slog.e(TAG, "Remote exception", e); - } - } - @Override // Binder call public long[] getAuthenticatorIds(int callingUserId) { checkInternalPermission(); 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 2c60d096bd136..cb7db92ee132e 100644 --- a/services/core/java/com/android/server/biometrics/sensors/ClientMonitorCallbackConverter.java +++ b/services/core/java/com/android/server/biometrics/sensors/ClientMonitorCallbackConverter.java @@ -128,11 +128,11 @@ public final class ClientMonitorCallbackConverter { } } - public void onChallengeGenerated(long challenge) throws RemoteException { + public void onChallengeGenerated(int sensorId, long challenge) throws RemoteException { if (mFaceServiceReceiver != null) { - mFaceServiceReceiver.onChallengeGenerated(challenge); + mFaceServiceReceiver.onChallengeGenerated(sensorId, challenge); } else if (mFingerprintServiceReceiver != null) { - mFingerprintServiceReceiver.onChallengeGenerated(challenge); + mFingerprintServiceReceiver.onChallengeGenerated(sensorId, challenge); } } diff --git a/services/core/java/com/android/server/biometrics/sensors/GenerateChallengeClient.java b/services/core/java/com/android/server/biometrics/sensors/GenerateChallengeClient.java index 2c5b8027f3099..92c498c55dbfd 100644 --- a/services/core/java/com/android/server/biometrics/sensors/GenerateChallengeClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/GenerateChallengeClient.java @@ -40,7 +40,7 @@ public abstract class GenerateChallengeClient extends ClientMonitor { @Override public void unableToStart() { try { - getListener().onChallengeGenerated(0L); + getListener().onChallengeGenerated(getSensorId(), 0L); } catch (RemoteException e) { Slog.e(TAG, "Unable to send error", e); } @@ -52,7 +52,7 @@ public abstract class GenerateChallengeClient extends ClientMonitor { startHalOperation(); try { - getListener().onChallengeGenerated(mChallenge); + getListener().onChallengeGenerated(getSensorId(), mChallenge); mCallback.onClientFinished(this, true /* success */); } catch (RemoteException e) { Slog.e(TAG, "Remote exception", e); 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 a4956cb565278..7dd6217016eb8 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 @@ -29,6 +29,7 @@ import android.hardware.biometrics.BiometricsProtoEnums; import android.hardware.biometrics.face.V1_0.IBiometricsFace; import android.hardware.biometrics.face.V1_0.IBiometricsFaceClientCallback; import android.hardware.face.Face; +import android.hardware.face.FaceManager; import android.hardware.face.FaceSensorProperties; import android.hardware.face.IFaceServiceReceiver; import android.os.Build; @@ -464,6 +465,21 @@ class Face10 implements IHwBinder.DeathRecipient { }); } + /** + * {@link IBiometricsFace} only supports a single in-flight challenge. In cases where two + * callers both need challenges (e.g. resetLockout right before enrollment), we need to ensure + * that either: + * 1) generateChallenge/operation/revokeChallenge is complete before the next generateChallenge + * is processed by the scheduler, or + * 2) the generateChallenge callback provides a mechanism for notifying the caller that its + * challenge has been invalidated by a subsequent caller, as well as a mechanism for + * notifying the previous caller that the interrupting operation is complete (e.g. the + * interrupting client's challenge has been revoked, so that the interrupted client can + * start retry logic if necessary). See + * {@link FaceManager.GenerateChallengeCallback#onChallengeInterruptFinished(int)} + * The only case of conflicting challenges is currently resetLockout --> enroll. So, the second + * option seems better as it prioritizes the new operation, which is user-facing. + */ void scheduleGenerateChallenge(@NonNull IBinder token, @NonNull IFaceServiceReceiver receiver, @NonNull String opPackageName) { mHandler.post(() -> { @@ -498,7 +514,8 @@ class Face10 implements IHwBinder.DeathRecipient { 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: " + Slog.e(TAG, "scheduleRevokeChallenge, package: " + owner + + " attempting to revoke challenge owned by: " + mCurrentChallengeOwner.getOwnerString()); return; } diff --git a/services/core/java/com/android/server/biometrics/sensors/face/FaceAuthenticator.java b/services/core/java/com/android/server/biometrics/sensors/face/FaceAuthenticator.java index 33244b8e61a82..bbee6cde4603c 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/FaceAuthenticator.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/FaceAuthenticator.java @@ -74,11 +74,6 @@ public final class FaceAuthenticator extends IBiometricAuthenticator.Stub { return mFaceService.getLockoutModeForUser(userId); } - @Override - public void resetLockout(int userId, byte[] hardwareAuthToken) throws RemoteException { - mFaceService.resetLockout(userId, hardwareAuthToken); - } - @Override public long getAuthenticatorId(int callingUserId) throws RemoteException { return mFaceService.getAuthenticatorId(callingUserId); 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 b832a09f8f883..b689106bbc442 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 @@ -80,16 +80,27 @@ public class FaceService extends SystemService { } @Override // Binder call - public void generateChallenge(IBinder token, IFaceServiceReceiver receiver, + public void generateChallenge(IBinder token, int sensorId, IFaceServiceReceiver receiver, String opPackageName) { Utils.checkPermission(getContext(), MANAGE_BIOMETRIC); - mFace10.scheduleGenerateChallenge(token, receiver, opPackageName); + if (sensorId == mFace10.getFaceSensorProperties().sensorId) { + mFace10.scheduleGenerateChallenge(token, receiver, opPackageName); + return; + } + + Slog.w(TAG, "No matching sensor for generateChallenge, sensorId: " + sensorId); } @Override // Binder call - public void revokeChallenge(IBinder token, String owner) { + public void revokeChallenge(IBinder token, int sensorId, String owner) { Utils.checkPermission(getContext(), MANAGE_BIOMETRIC); - mFace10.scheduleRevokeChallenge(token, owner); + + if (sensorId == mFace10.getFaceSensorProperties().sensorId) { + mFace10.scheduleRevokeChallenge(token, owner); + return; + } + + Slog.w(TAG, "No matching sensor for revokeChallenge, sensorId: " + sensorId); } @Override // Binder call @@ -267,9 +278,16 @@ public class FaceService extends SystemService { } @Override // Binder call - public void resetLockout(int userId, byte[] hardwareAuthToken) { + public void resetLockout(IBinder token, int sensorId, int userId, byte[] hardwareAuthToken, + String opPackageName) { Utils.checkPermission(getContext(), USE_BIOMETRIC_INTERNAL); - mFace10.scheduleResetLockout(userId, hardwareAuthToken); + + if (sensorId == mFace10.getFaceSensorProperties().sensorId) { + mFace10.scheduleResetLockout(userId, hardwareAuthToken); + return; + } + + Slog.w(TAG, "No matching sensor for resetLockout, sensorId: " + sensorId); } @Override diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/Fingerprint21.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/Fingerprint21.java index a12b54cfdd7f7..c5c28227fd245 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/Fingerprint21.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/Fingerprint21.java @@ -336,7 +336,10 @@ class Fingerprint21 implements IHwBinder.DeathRecipient { final @FingerprintSensorProperties.SensorType int sensorType = isUdfps ? FingerprintSensorProperties.TYPE_UDFPS : FingerprintSensorProperties.TYPE_REAR; - mSensorProperties = new FingerprintSensorProperties(sensorId, sensorType); + // resetLockout is controlled by the framework, so hardwareAuthToken is not required + final boolean resetLockoutRequiresHardwareAuthToken = false; + mSensorProperties = new FingerprintSensorProperties(sensorId, sensorType, + resetLockoutRequiresHardwareAuthToken); } static Fingerprint21 newInstance(@NonNull Context context, int sensorId, @@ -469,7 +472,7 @@ class Fingerprint21 implements IHwBinder.DeathRecipient { }); } - void scheduleResetLockout(int userId, byte[] hardwareAuthToken) { + void scheduleResetLockout(int userId) { // Fingerprint2.1 keeps track of lockout in the framework. Let's just do it on the handler // thread. mHandler.post(() -> { diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/Fingerprint21UdfpsMock.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/Fingerprint21UdfpsMock.java index c5567744851f3..1a6ad46fce673 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/Fingerprint21UdfpsMock.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/Fingerprint21UdfpsMock.java @@ -406,8 +406,10 @@ public class Fingerprint21UdfpsMock extends Fingerprint21 implements TrustManage mScheduler = scheduler; mScheduler.init(this); mHandler = handler; + // resetLockout is controlled by the framework, so hardwareAuthToken is not required + final boolean resetLockoutRequiresHardwareAuthToken = false; mSensorProperties = new FingerprintSensorProperties(sensorId, - FingerprintSensorProperties.TYPE_UDFPS); + FingerprintSensorProperties.TYPE_UDFPS, resetLockoutRequiresHardwareAuthToken); mMockHalResultController = controller; mUserHasTrust = new SparseBooleanArray(); mTrustManager = context.getSystemService(TrustManager.class); diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintAuthenticator.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintAuthenticator.java index 3418c466aa69a..21a46d58a3b34 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintAuthenticator.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/FingerprintAuthenticator.java @@ -74,11 +74,6 @@ public final class FingerprintAuthenticator extends IBiometricAuthenticator.Stub return mFingerprintService.getLockoutModeForUser(userId); } - @Override - public void resetLockout(int userId, byte[] hardwareAuthToken) throws RemoteException { - mFingerprintService.resetLockout(userId, hardwareAuthToken); - } - @Override public long getAuthenticatorId(int callingUserId) throws RemoteException { return mFingerprintService.getAuthenticatorId(callingUserId); 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 be8528c435f5b..7c7da118df106 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 @@ -25,6 +25,7 @@ import static android.Manifest.permission.USE_BIOMETRIC_INTERNAL; import static android.Manifest.permission.USE_FINGERPRINT; import android.annotation.NonNull; +import android.annotation.Nullable; import android.app.AppOpsManager; import android.content.Context; import android.content.pm.PackageManager; @@ -97,10 +98,16 @@ public class FingerprintService extends SystemService { } @Override // Binder call - public void generateChallenge(IBinder token, IFingerprintServiceReceiver receiver, - String opPackageName) { + public void generateChallenge(IBinder token, int sensorId, + IFingerprintServiceReceiver receiver, String opPackageName) { Utils.checkPermission(getContext(), MANAGE_FINGERPRINT); - mFingerprint21.scheduleGenerateChallenge(token, receiver, opPackageName); + + if (sensorId == mFingerprint21.getFingerprintSensorProperties().sensorId) { + mFingerprint21.scheduleGenerateChallenge(token, receiver, opPackageName); + return; + } + + Slog.w(TAG, "No matching sensor for generateChallenge, sensorId: " + sensorId); } @Override // Binder call @@ -346,9 +353,16 @@ public class FingerprintService extends SystemService { } @Override // Binder call - public void resetLockout(int userId, byte [] hardwareAuthToken) { + public void resetLockout(IBinder token, int sensorId, int userId, + @Nullable byte [] hardwareAuthToken, String opPackageName) { Utils.checkPermission(getContext(), RESET_FINGERPRINT_LOCKOUT); - mFingerprint21.scheduleResetLockout(userId, hardwareAuthToken); + + if (sensorId == mFingerprint21.getFingerprintSensorProperties().sensorId) { + mFingerprint21.scheduleResetLockout(userId); + return; + } + + Slog.w(TAG, "No matching sensor for resetLockout, sensorId: " + sensorId); } @Override diff --git a/services/core/java/com/android/server/biometrics/sensors/iris/IrisAuthenticator.java b/services/core/java/com/android/server/biometrics/sensors/iris/IrisAuthenticator.java index 9e0405792746c..0400ef522142a 100644 --- a/services/core/java/com/android/server/biometrics/sensors/iris/IrisAuthenticator.java +++ b/services/core/java/com/android/server/biometrics/sensors/iris/IrisAuthenticator.java @@ -69,10 +69,6 @@ public final class IrisAuthenticator extends IBiometricAuthenticator.Stub { return LockoutTracker.LOCKOUT_NONE; } - @Override - public void resetLockout(int userId, byte[] hardwareAuthToken) throws RemoteException { - } - @Override public long getAuthenticatorId(int callingUserId) throws RemoteException { return 0; diff --git a/services/core/java/com/android/server/locksettings/BiometricDeferredQueue.java b/services/core/java/com/android/server/locksettings/BiometricDeferredQueue.java new file mode 100644 index 0000000000000..5573eede0d40e --- /dev/null +++ b/services/core/java/com/android/server/locksettings/BiometricDeferredQueue.java @@ -0,0 +1,252 @@ +/* + * Copyright (C) 2020 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 com.android.server.locksettings; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.content.Context; +import android.hardware.face.FaceManager; +import android.hardware.face.FaceSensorProperties; +import android.hardware.fingerprint.FingerprintManager; +import android.hardware.fingerprint.FingerprintSensorProperties; +import android.os.Handler; +import android.os.IBinder; +import android.os.ServiceManager; +import android.service.gatekeeper.IGateKeeperService; +import android.util.ArraySet; +import android.util.Slog; + +import com.android.internal.widget.VerifyCredentialResponse; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * Class that handles biometric-related work in the {@link LockSettingsService} area, for example + * resetLockout. + */ +@SuppressWarnings("deprecation") +public class BiometricDeferredQueue { + private static final String TAG = "BiometricDeferredQueue"; + + @NonNull private final Context mContext; + @NonNull private final SyntheticPasswordManager mSpManager; + @NonNull private final Handler mHandler; + @Nullable private FingerprintManager mFingerprintManager; + @Nullable private FaceManager mFaceManager; + + // Entries added by LockSettingsService once a user's synthetic password is known. At this point + // things are still keyed by userId. + @NonNull private final ArrayList mPendingResetLockouts; + + /** + * Authentication info for a successful user unlock via Synthetic Password. This can be used to + * perform multiple operations (e.g. resetLockout for multiple HALs/Sensors) by sending the + * Gatekeeper Password to Gatekeer multiple times, each with a sensor-specific challenge. + */ + private static class UserAuthInfo { + final int userId; + @NonNull final byte[] gatekeeperPassword; + + UserAuthInfo(int userId, @NonNull byte[] gatekeeperPassword) { + this.userId = userId; + this.gatekeeperPassword = gatekeeperPassword; + } + } + + /** + * Per-authentication callback. + */ + private static class FaceResetLockoutTask implements FaceManager.GenerateChallengeCallback { + interface FinishCallback { + void onFinished(); + } + + @NonNull FinishCallback finishCallback; + @NonNull FaceManager faceManager; + @NonNull SyntheticPasswordManager spManager; + @NonNull Set sensorIds; // IDs of sensors waiting for challenge + @NonNull List pendingResetLockuts; + + FaceResetLockoutTask( + @NonNull FinishCallback finishCallback, + @NonNull FaceManager faceManager, + @NonNull SyntheticPasswordManager spManager, + @NonNull Set sensorIds, + @NonNull List pendingResetLockouts) { + this.finishCallback = finishCallback; + this.faceManager = faceManager; + this.spManager = spManager; + this.sensorIds = sensorIds; + this.pendingResetLockuts = pendingResetLockouts; + } + + @Override + public void onChallengeInterrupted(int sensorId) { + Slog.w(TAG, "Challenge interrupted, sensor: " + sensorId); + // Consider re-attempting generateChallenge/resetLockout/revokeChallenge + // when onChallengeInterruptFinished is invoked + } + + @Override + public void onChallengeInterruptFinished(int sensorId) { + Slog.w(TAG, "Challenge interrupt finished, sensor: " + sensorId); + } + + @Override + public void onGenerateChallengeResult(int sensorId, long challenge) { + if (!sensorIds.contains(sensorId)) { + Slog.e(TAG, "Unknown sensorId received: " + sensorId); + return; + } + + // Challenge received for a sensor. For each sensor, reset lockout for all users. + for (UserAuthInfo userAuthInfo : pendingResetLockuts) { + Slog.d(TAG, "Resetting face lockout for sensor: " + sensorId + + ", user: " + userAuthInfo.userId); + final VerifyCredentialResponse response = spManager.verifyChallengeInternal( + getGatekeeperService(), userAuthInfo.gatekeeperPassword, challenge, + userAuthInfo.userId); + if (response.getResponseCode() != VerifyCredentialResponse.RESPONSE_OK) { + Slog.wtf(TAG, "VerifyChallenge failed, response: " + + response.getResponseCode()); + } + faceManager.resetLockout(sensorId, userAuthInfo.userId, + response.getGatekeeperHAT()); + } + + sensorIds.remove(sensorId); + faceManager.revokeChallenge(sensorId); + + if (sensorIds.isEmpty()) { + Slog.d(TAG, "Done requesting resetLockout for all face sensors"); + finishCallback.onFinished(); + } + } + + synchronized IGateKeeperService getGatekeeperService() { + final IBinder service = ServiceManager.getService(Context.GATEKEEPER_SERVICE); + if (service == null) { + Slog.e(TAG, "Unable to acquire GateKeeperService"); + return null; + } + return IGateKeeperService.Stub.asInterface(service); + } + } + + @Nullable private FaceResetLockoutTask mFaceResetLockoutTask; + + private final FaceResetLockoutTask.FinishCallback mFaceFinishCallback = () -> { + mFaceResetLockoutTask = null; + }; + + BiometricDeferredQueue(@NonNull Context context, @NonNull SyntheticPasswordManager spManager, + @NonNull Handler handler) { + mContext = context; + mSpManager = spManager; + mHandler = handler; + mPendingResetLockouts = new ArrayList<>(); + } + + public void systemReady() { + mFingerprintManager = mContext.getSystemService(FingerprintManager.class); + mFaceManager = mContext.getSystemService(FaceManager.class); + } + + /** + * Adds a request for resetLockout on all biometric sensors for the user specified. The queue + * owner must invoke {@link #processPendingLockoutResets()} at some point to kick off the + * operations. + * + * Note that this should only ever be invoked for successful authentications, otherwise it will + * consume a Gatekeeper authentication attempt and potentially wipe the user/device. + * + * @param userId The user that the operation will apply for. + * @param gatekeeperPassword The Gatekeeper Password + */ + void addPendingLockoutResetForUser(int userId, @NonNull byte[] gatekeeperPassword) { + mHandler.post(() -> { + Slog.d(TAG, "addPendingLockoutResetForUser: " + userId); + mPendingResetLockouts.add(new UserAuthInfo(userId, gatekeeperPassword)); + }); + } + + void processPendingLockoutResets() { + mHandler.post(() -> { + Slog.d(TAG, "processPendingLockoutResets: " + mPendingResetLockouts.size()); + processPendingLockoutsForFingerprint(new ArrayList<>(mPendingResetLockouts)); + processPendingLockoutsForFace(new ArrayList<>(mPendingResetLockouts)); + mPendingResetLockouts.clear(); + }); + } + + private void processPendingLockoutsForFingerprint(List pendingResetLockouts) { + if (mFingerprintManager != null) { + final List fingerprintSensorProperties = + mFingerprintManager.getSensorProperties(); + for (FingerprintSensorProperties prop : fingerprintSensorProperties) { + if (!prop.resetLockoutRequiresHardwareAuthToken) { + for (UserAuthInfo user : pendingResetLockouts) { + mFingerprintManager.resetLockout(prop.sensorId, user.userId, + null /* hardwareAuthToken */); + } + } else { + Slog.e(TAG, "Fingerprint resetLockout with HAT not supported yet"); + // TODO(b/152414803): Implement this when resetLockout is implemented below + // the framework. + } + } + } + } + + /** + * For devices on {@link android.hardware.biometrics.face.V1_0} which only support a single + * in-flight challenge, we generate a single challenge to reset lockout for all profiles. This + * hopefully reduces/eliminates issues such as overwritten challenge, incorrectly revoked + * challenge, or other race conditions. + * + * TODO(b/162965646) This logic can be avoided if multiple in-flight challenges are supported. + * Though it will need to continue to exist to support existing HIDLs, each profile that + * requires resetLockout could have its own challenge, and the `mPendingResetLockouts` queue + * can be avoided. + */ + private void processPendingLockoutsForFace(List pendingResetLockouts) { + if (mFaceManager != null) { + if (mFaceResetLockoutTask != null) { + // This code will need to be updated if this problem ever occurs. + Slog.w(TAG, "mFaceGenerateChallengeCallback not null, previous operation may be" + + " stuck"); + } + final List faceSensorProperties = + mFaceManager.getSensorProperties(); + final Set sensorIds = new ArraySet<>(); + for (FaceSensorProperties prop : faceSensorProperties) { + sensorIds.add(prop.sensorId); + } + + mFaceResetLockoutTask = new FaceResetLockoutTask(mFaceFinishCallback, mFaceManager, + mSpManager, sensorIds, pendingResetLockouts); + for (final FaceSensorProperties prop : faceSensorProperties) { + // Generate a challenge for each sensor. The challenge does not need to be + // per-user, since the HAT returned by gatekeeper contains userId. + mFaceManager.generateChallenge(prop.sensorId, mFaceResetLockoutTask); + } + } + } +} diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index d6e37bacdba8a..6eaa5faac8cba 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -37,7 +37,6 @@ import static com.android.internal.widget.LockPatternUtils.VERIFY_FLAG_RETURN_GK import static com.android.internal.widget.LockPatternUtils.frpCredentialEnabled; import static com.android.internal.widget.LockPatternUtils.userOwnsFrpCredential; -import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.UserIdInt; @@ -187,23 +186,6 @@ public class LockSettingsService extends ILockSettings.Stub { private static final String SYNTHETIC_PASSWORD_UPDATE_TIME_KEY = "sp-handle-ts"; private static final String USER_SERIAL_NUMBER_KEY = "serial-number"; - // TODO (b/145978626) LockSettingsService no longer accepts challenges in the verifyCredential - // paths. These are temporarily left around to ensure that resetLockout works. It will be - // removed once resetLockout is compartmentalized. - // No challenge provided - private static final int CHALLENGE_NONE = 0; - // Challenge was provided from the external caller (non-LockSettingsService) - private static final int CHALLENGE_FROM_CALLER = 1; - // Challenge was generated from within LockSettingsService, for resetLockout. When challenge - // type is set to internal, LSS will revokeChallenge after all profiles for that user are - // unlocked. - private static final int CHALLENGE_INTERNAL = 2; - - @IntDef({CHALLENGE_NONE, - CHALLENGE_FROM_CALLER, - CHALLENGE_INTERNAL}) - @interface ChallengeType {} - // Order of holding lock: mSeparateChallengeLock -> mSpManager -> this // Do not call into ActivityManager while holding mSpManager lock. private final Object mSeparateChallengeLock = new Object(); @@ -219,6 +201,7 @@ public class LockSettingsService extends ILockSettings.Stub { protected final LockSettingsStorage mStorage; private final LockSettingsStrongAuth mStrongAuth; private final SynchronizedStrongAuthTracker mStrongAuthTracker; + private final BiometricDeferredQueue mBiometricDeferredQueue; private final NotificationManager mNotificationManager; private final UserManager mUserManager; @@ -321,15 +304,6 @@ public class LockSettingsService extends ILockSettings.Stub { } } - private class PendingResetLockout { - final int mUserId; - final byte[] mHAT; - PendingResetLockout(int userId, byte[] hat) { - mUserId = userId; - mHAT = hat; - } - } - private LockscreenCredential generateRandomProfilePassword() { byte[] randomLockSeed = new byte[] {}; try { @@ -588,6 +562,7 @@ public class LockSettingsService extends ILockSettings.Stub { mSpManager = injector.getSyntheticPasswordManager(mStorage); mManagedProfilePasswordCache = injector.getManagedProfilePasswordCache(); + mBiometricDeferredQueue = new BiometricDeferredQueue(mContext, mSpManager, mHandler); mRebootEscrowManager = injector.getRebootEscrowManager(new RebootEscrowCallbacks(), mStorage); @@ -740,8 +715,7 @@ public class LockSettingsService extends ILockSettings.Stub { // If boot took too long and the password in vold got expired, parent keystore will // be still locked, we ignore this case since the user will be prompted to unlock // the device after boot. - unlockChildProfile(userId, true /* ignoreUserNotAuthenticated */, - CHALLENGE_NONE, 0 /* challenge */, null /* resetLockouts */); + unlockChildProfile(userId, true /* ignoreUserNotAuthenticated */); } } @@ -830,6 +804,7 @@ public class LockSettingsService extends ILockSettings.Stub { mRebootEscrowManager.loadRebootEscrowDataIfAvailable(); // TODO: maybe skip this for split system user mode. mStorage.prefetchUser(UserHandle.USER_SYSTEM); + mBiometricDeferredQueue.systemReady(); } private void getAuthSecretHal() { @@ -1306,13 +1281,10 @@ public class LockSettingsService extends ILockSettings.Stub { return credential; } - private void unlockChildProfile(int profileHandle, boolean ignoreUserNotAuthenticated, - @ChallengeType int challengeType, long challenge, - @Nullable ArrayList resetLockouts) { + private void unlockChildProfile(int profileHandle, boolean ignoreUserNotAuthenticated) { try { doVerifyCredential(getDecryptedPasswordForTiedProfile(profileHandle), - challengeType, challenge, profileHandle, null /* progressCallback */, - resetLockouts, 0 /* flags */); + profileHandle, null /* progressCallback */, 0 /* flags */); } catch (UnrecoverableKeyException | InvalidKeyException | KeyStoreException | NoSuchAlgorithmException | NoSuchPaddingException | InvalidAlgorithmParameterException | IllegalBlockSizeException @@ -1327,10 +1299,6 @@ public class LockSettingsService extends ILockSettings.Stub { } } - private void unlockUser(int userId, byte[] token, byte[] secret) { - unlockUser(userId, token, secret, CHALLENGE_NONE, 0 /* challenge */, null); - } - /** * Unlock the user (both storage and user state) and its associated managed profiles * synchronously. @@ -1339,9 +1307,7 @@ public class LockSettingsService extends ILockSettings.Stub { * can end up calling into other system services to process user unlock request (via * {@link com.android.server.SystemServiceManager#unlockUser} */ - private void unlockUser(int userId, byte[] token, byte[] secret, - @ChallengeType int challengeType, long challenge, - @Nullable ArrayList resetLockouts) { + private void unlockUser(int userId, byte[] token, byte[] secret) { Slog.i(TAG, "Unlocking user " + userId + " with secret only, length " + (secret != null ? secret.length : 0)); // TODO: make this method fully async so we can update UI with progress strings @@ -1378,6 +1344,9 @@ public class LockSettingsService extends ILockSettings.Stub { } if (mUserManager.getUserInfo(userId).isManagedProfile()) { + if (!hasUnifiedChallenge(userId)) { + mBiometricDeferredQueue.processPendingLockoutResets(); + } return; } @@ -1388,10 +1357,7 @@ public class LockSettingsService extends ILockSettings.Stub { if (hasUnifiedChallenge(profile.id)) { if (mUserManager.isUserRunning(profile.id)) { // Unlock managed profile with unified lock - // Must pass the challenge on for resetLockout, so it's not over-written, which - // causes LockSettingsService to revokeChallenge inappropriately. - unlockChildProfile(profile.id, false /* ignoreUserNotAuthenticated */, - challengeType, challenge, resetLockouts); + unlockChildProfile(profile.id, false /* ignoreUserNotAuthenticated */); } else { try { // Profile not ready for unlock yet, but decrypt the unified challenge now @@ -1412,22 +1378,9 @@ public class LockSettingsService extends ILockSettings.Stub { restoreCallingIdentity(ident); } } - } - if (resetLockouts != null && !resetLockouts.isEmpty()) { - mHandler.post(() -> { - final BiometricManager bm = mContext.getSystemService(BiometricManager.class); - final PackageManager pm = mContext.getPackageManager(); - for (int i = 0; i < resetLockouts.size(); i++) { - bm.resetLockout(resetLockouts.get(i).mUserId, resetLockouts.get(i).mHAT); - } - if (challengeType == CHALLENGE_INTERNAL - && pm.hasSystemFeature(PackageManager.FEATURE_FACE)) { - mContext.getSystemService(FaceManager.class).revokeChallenge(); - } - }); - } + mBiometricDeferredQueue.processPendingLockoutResets(); } private boolean hasUnifiedChallenge(int userId) { @@ -1727,8 +1680,7 @@ public class LockSettingsService extends ILockSettings.Stub { setUserKeyProtection(userId, credential, convertResponse(gkResponse)); fixateNewestUserKeyAuth(userId); // Refresh the auth token - doVerifyCredential(credential, CHALLENGE_FROM_CALLER, 0, userId, - null /* progressCallback */, 0 /* flags */); + doVerifyCredential(credential, userId, null /* progressCallback */, 0 /* flags */); synchronizeUnifiedWorkChallengeForProfiles(userId, null); sendCredentialsOnChangeIfRequired(credential, userId, isLockTiedToParent); return true; @@ -1972,8 +1924,7 @@ public class LockSettingsService extends ILockSettings.Stub { ICheckCredentialProgressCallback progressCallback) { checkPasswordReadPermission(userId); try { - return doVerifyCredential(credential, CHALLENGE_NONE, 0L, userId, progressCallback, - 0 /* flags */); + return doVerifyCredential(credential, userId, progressCallback, 0 /* flags */); } finally { scheduleGc(); } @@ -1984,10 +1935,8 @@ public class LockSettingsService extends ILockSettings.Stub { public VerifyCredentialResponse verifyCredential(LockscreenCredential credential, int userId, int flags) { checkPasswordReadPermission(userId); - try { - return doVerifyCredential(credential, CHALLENGE_NONE, 0L, userId, - null /* progressCallback */, flags); + return doVerifyCredential(credential, userId, null /* progressCallback */, flags); } finally { scheduleGc(); } @@ -2006,32 +1955,17 @@ public class LockSettingsService extends ILockSettings.Stub { return response; } - /** + /* + * Verify user credential and unlock the user. Fix pattern bug by deprecating the old base zero + * format. * @param credential User's lockscreen credential - * @param challengeType Owner of the challenge - * @param challenge Challenge to be wrapped within Gatekeeper's HAT, if the credential is - * verified * @param userId User to verify the credential for * @param progressCallback Receive progress callbacks * @param flags See {@link LockPatternUtils.VerifyFlag} * @return See {@link VerifyCredentialResponse} */ private VerifyCredentialResponse doVerifyCredential(LockscreenCredential credential, - @ChallengeType int challengeType, long challenge, int userId, - ICheckCredentialProgressCallback progressCallback, - @LockPatternUtils.VerifyFlag int flags) { - return doVerifyCredential(credential, challengeType, challenge, userId, - progressCallback, null /* resetLockouts */, flags); - } - - /** - * Verify user credential and unlock the user. Fix pattern bug by deprecating the old base zero - * format. - */ - private VerifyCredentialResponse doVerifyCredential(LockscreenCredential credential, - @ChallengeType int challengeType, long challenge, int userId, - ICheckCredentialProgressCallback progressCallback, - @Nullable ArrayList resetLockouts, + int userId, ICheckCredentialProgressCallback progressCallback, @LockPatternUtils.VerifyFlag int flags) { if (credential == null || credential.isNone()) { throw new IllegalArgumentException("Credential can't be null or empty"); @@ -2041,9 +1975,10 @@ public class LockSettingsService extends ILockSettings.Stub { Slog.e(TAG, "FRP credential can only be verified prior to provisioning."); return VerifyCredentialResponse.ERROR; } - VerifyCredentialResponse response = null; - response = spBasedDoVerifyCredential(credential, challengeType, challenge, - userId, progressCallback, resetLockouts, flags); + + VerifyCredentialResponse response = spBasedDoVerifyCredential(credential, userId, + progressCallback, flags); + // The user employs synthetic password based credential. if (response != null) { if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { @@ -2064,8 +1999,7 @@ public class LockSettingsService extends ILockSettings.Stub { return VerifyCredentialResponse.ERROR; } - response = verifyCredential(userId, storedHash, credential, - challengeType, challenge, progressCallback); + response = verifyCredential(userId, storedHash, credential, progressCallback); if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { mStrongAuth.reportSuccessfulStrongAuthUnlock(userId); @@ -2085,8 +2019,6 @@ public class LockSettingsService extends ILockSettings.Stub { // Unlock parent by using parent's challenge final VerifyCredentialResponse parentResponse = doVerifyCredential( credential, - CHALLENGE_NONE, - 0L, parentProfileId, null /* progressCallback */, flags); @@ -2098,10 +2030,7 @@ public class LockSettingsService extends ILockSettings.Stub { try { // Unlock work profile, and work profile with unified lock must use password only return doVerifyCredential(getDecryptedPasswordForTiedProfile(userId), - CHALLENGE_NONE, - 0L, - userId, null /* progressCallback */, - flags); + userId, null /* progressCallback */, flags); } catch (UnrecoverableKeyException | InvalidKeyException | KeyStoreException | NoSuchAlgorithmException | NoSuchPaddingException | InvalidAlgorithmParameterException | IllegalBlockSizeException @@ -2119,8 +2048,7 @@ public class LockSettingsService extends ILockSettings.Stub { * hash to GK. */ private VerifyCredentialResponse verifyCredential(int userId, CredentialHash storedHash, - LockscreenCredential credential, @ChallengeType int challengeType, long challenge, - ICheckCredentialProgressCallback progressCallback) { + LockscreenCredential credential, ICheckCredentialProgressCallback progressCallback) { if ((storedHash == null || storedHash.hash.length == 0) && credential.isNone()) { // don't need to pass empty credentials to GateKeeper return VerifyCredentialResponse.OK; @@ -2137,7 +2065,7 @@ public class LockSettingsService extends ILockSettings.Stub { GateKeeperResponse gateKeeperResponse; try { gateKeeperResponse = getGateKeeperService().verifyChallenge( - userId, challenge, storedHash.hash, credential.getCredential()); + userId, 0L /* challenge */, storedHash.hash, credential.getCredential()); } catch (RemoteException e) { Slog.e(TAG, "gatekeeper verify failed", e); gateKeeperResponse = GateKeeperResponse.ERROR; @@ -2710,28 +2638,13 @@ public class LockSettingsService extends ILockSettings.Stub { } private VerifyCredentialResponse spBasedDoVerifyCredential(LockscreenCredential userCredential, - @ChallengeType int challengeType, long challenge, int userId, ICheckCredentialProgressCallback progressCallback, - @Nullable ArrayList resetLockouts, @LockPatternUtils.VerifyFlag int flags) { - final boolean hasEnrolledBiometrics = mInjector.hasEnrolledBiometrics(userId); - Slog.d(TAG, "spBasedDoVerifyCredential: user=" + userId + " challengeType=" + challengeType + Slog.d(TAG, "spBasedDoVerifyCredential: user=" + userId + " hasEnrolledBiometrics=" + hasEnrolledBiometrics); - final PackageManager pm = mContext.getPackageManager(); - // TODO: When lockout is handled under the HAL for all biometrics (fingerprint), - // we need to generate challenge for each one, have it signed by GK and reset lockout - // for each modality. - if (challengeType == CHALLENGE_NONE && pm.hasSystemFeature(PackageManager.FEATURE_FACE) - && hasEnrolledBiometrics) { - // If there are multiple profiles in the same account, ensure we only generate the - // challenge once. - challengeType = CHALLENGE_INTERNAL; - challenge = mContext.getSystemService(FaceManager.class).generateChallengeBlocking(); - } - final AuthenticationResult authResult; VerifyCredentialResponse response; final boolean returnGkPw = (flags & VERIFY_FLAG_RETURN_GK_PW) != 0; @@ -2752,10 +2665,13 @@ public class LockSettingsService extends ILockSettings.Stub { // credential has matched if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { + mBiometricDeferredQueue.addPendingLockoutResetForUser(userId, + authResult.authToken.deriveGkPassword()); + // perform verifyChallenge with synthetic password which generates the real GK auth // token and response for the current user response = mSpManager.verifyChallenge(getGateKeeperService(), authResult.authToken, - challenge, userId); + 0L /* challenge */, userId); if (response.getResponseCode() != VerifyCredentialResponse.RESPONSE_OK) { // This shouldn't really happen: the unwrapping of SP succeeds, but SP doesn't // match the recorded GK password handle. @@ -2765,15 +2681,7 @@ public class LockSettingsService extends ILockSettings.Stub { } } if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_OK) { - // Do resetLockout / revokeChallenge when all profiles are unlocked - if (hasEnrolledBiometrics) { - if (resetLockouts == null) { - resetLockouts = new ArrayList<>(); - } - resetLockouts.add(new PendingResetLockout(userId, response.getGatekeeperHAT())); - } - - onCredentialVerified(authResult.authToken, challengeType, challenge, resetLockouts, + onCredentialVerified(authResult.authToken, PasswordMetrics.computeForCredential(userCredential), userId); } else if (response.getResponseCode() == VerifyCredentialResponse.RESPONSE_RETRY) { if (response.getTimeout() > 0) { @@ -2789,9 +2697,7 @@ public class LockSettingsService extends ILockSettings.Stub { } } - private void onCredentialVerified(AuthenticationToken authToken, - @ChallengeType int challengeType, long challenge, - @Nullable ArrayList resetLockouts, PasswordMetrics metrics, + private void onCredentialVerified(AuthenticationToken authToken, PasswordMetrics metrics, int userId) { if (metrics != null) { @@ -2806,7 +2712,7 @@ public class LockSettingsService extends ILockSettings.Stub { { final byte[] secret = authToken.deriveDiskEncryptionKey(); - unlockUser(userId, null, secret, challengeType, challenge, resetLockouts); + unlockUser(userId, null, secret); Arrays.fill(secret, (byte) 0); } activateEscrowTokens(authToken, userId); @@ -3193,11 +3099,8 @@ public class LockSettingsService extends ILockSettings.Stub { return false; } } - // TODO: Reset biometrics lockout here. Ideally that should be self-contained inside - // onCredentialVerified(), which will require some refactoring on the current lockout - // reset logic. - onCredentialVerified(authResult.authToken, CHALLENGE_NONE, 0, null, + onCredentialVerified(authResult.authToken, loadPasswordMetrics(authResult.authToken, userId), userId); return true; } @@ -3208,8 +3111,7 @@ public class LockSettingsService extends ILockSettings.Stub { if (cred == null) { return false; } - return doVerifyCredential(cred, CHALLENGE_NONE, 0, userId, - null /* progressCallback */, 0 /* flags */) + return doVerifyCredential(cred, userId, null /* progressCallback */, 0 /* flags */) .getResponseCode() == VerifyCredentialResponse.RESPONSE_OK; } } @@ -3532,8 +3434,7 @@ public class LockSettingsService extends ILockSettings.Stub { SyntheticPasswordManager.AuthenticationToken authToken = new SyntheticPasswordManager.AuthenticationToken(spVersion); authToken.recreateDirectly(syntheticPassword); - onCredentialVerified(authToken, CHALLENGE_NONE, 0, null, - loadPasswordMetrics(authToken, userId), userId); + onCredentialVerified(authToken, loadPasswordMetrics(authToken, userId), userId); } } } diff --git a/services/tests/servicestests/src/com/android/server/biometrics/AuthServiceTest.java b/services/tests/servicestests/src/com/android/server/biometrics/AuthServiceTest.java index a5df53205a367..94cad1ed18b89 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/AuthServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/AuthServiceTest.java @@ -269,21 +269,6 @@ public class AuthServiceTest { eq(callback), eq(UserHandle.getCallingUserId())); } - @Test - public void testResetLockout_callsBiometricServiceResetLockout() throws - Exception { - mAuthService = new AuthService(mContext, mInjector); - mAuthService.onStart(); - - final int userId = 100; - final byte[] token = new byte[0]; - - mAuthService.mImpl.resetLockout(userId, token); - - waitForIdle(); - verify(mBiometricService).resetLockout(eq(userId), AdditionalMatchers.aryEq(token)); - } - private static void waitForIdle() { InstrumentationRegistry.getInstrumentation().waitForIdleSync(); }