From 72c34460490e09789969d82b2d987a89affeb5bf Mon Sep 17 00:00:00 2001 From: Joe Bolinger Date: Fri, 7 May 2021 17:53:09 -0700 Subject: [PATCH] Add userId parameter to all uses of challenge client. Bug: 171335732 Bug: 186310980 Fix: 184915229 Test: atest com.android.server.biometrics Test: manually on device (as normal, extra user, & work profile) Change-Id: Iccb65657736a73c591a3bb5b69fb432d3f0cb676 --- .../android/hardware/face/FaceManager.java | 36 ++++++++++--------- .../hardware/face/FaceServiceReceiver.java | 3 +- .../hardware/face/IFaceServiceReceiver.aidl | 2 +- .../fingerprint/FingerprintManager.java | 16 +++++---- .../FingerprintServiceReceiver.java | 3 +- .../IFingerprintServiceReceiver.aidl | 2 +- .../biometrics/sensors/BaseClientMonitor.java | 3 +- .../ClientMonitorCallbackConverter.java | 8 +++-- .../sensors/GenerateChallengeClient.java | 2 +- .../face/aidl/BiometricTestSessionImpl.java | 2 +- .../aidl/FaceGenerateChallengeClient.java | 2 +- .../face/hidl/BiometricTestSessionImpl.java | 2 +- .../hidl/FaceGenerateChallengeClient.java | 2 +- .../aidl/BiometricTestSessionImpl.java | 2 +- .../FingerprintGenerateChallengeClient.java | 2 +- .../hidl/BiometricTestSessionImpl.java | 2 +- .../FingerprintGenerateChallengeClient.java | 2 +- .../locksettings/BiometricDeferredQueue.java | 29 ++++++--------- .../hidl/FaceGenerateChallengeClientTest.java | 6 ++-- 19 files changed, 64 insertions(+), 62 deletions(-) diff --git a/core/java/android/hardware/face/FaceManager.java b/core/java/android/hardware/face/FaceManager.java index 7ac3d3c743745..55c90ce2a32f1 100644 --- a/core/java/android/hardware/face/FaceManager.java +++ b/core/java/android/hardware/face/FaceManager.java @@ -100,8 +100,8 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan @Override // binder call public void onAuthenticationSucceeded(Face face, int userId, boolean isStrongBiometric) { - mHandler.obtainMessage(MSG_AUTHENTICATION_SUCCEEDED, userId, isStrongBiometric ? 1 : 0, - face).sendToTarget(); + mHandler.obtainMessage(MSG_AUTHENTICATION_SUCCEEDED, userId, + isStrongBiometric ? 1 : 0, face).sendToTarget(); } @Override // binder call @@ -140,8 +140,8 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } @Override - public void onChallengeGenerated(int sensorId, long challenge) { - mHandler.obtainMessage(MSG_CHALLENGE_GENERATED, sensorId, 0, challenge) + public void onChallengeGenerated(int sensorId, int userId, long challenge) { + mHandler.obtainMessage(MSG_CHALLENGE_GENERATED, sensorId, userId, challenge) .sendToTarget(); } @@ -422,16 +422,14 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan * * @see com.android.server.locksettings.LockSettingsService * - * TODO(b/171335732): should take userId - * * @hide */ @RequiresPermission(MANAGE_BIOMETRIC) - public void generateChallenge(int sensorId, GenerateChallengeCallback callback) { + public void generateChallenge(int sensorId, int userId, GenerateChallengeCallback callback) { if (mService != null) { try { mGenerateChallengeCallback = callback; - mService.generateChallenge(mToken, sensorId, 0 /* userId */, mServiceReceiver, + mService.generateChallenge(mToken, sensorId, userId, mServiceReceiver, mContext.getOpPackageName()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); @@ -440,12 +438,13 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } /** - * Same as {@link #generateChallenge(int, GenerateChallengeCallback)}, but assumes the first - * enumerated sensor. + * Same as {@link #generateChallenge(int, int, GenerateChallengeCallback)}, but assumes the + * first enumerated sensor. + * * @hide */ @RequiresPermission(MANAGE_BIOMETRIC) - public void generateChallenge(GenerateChallengeCallback callback) { + public void generateChallenge(int userId, GenerateChallengeCallback callback) { final List faceSensorProperties = getSensorPropertiesInternal(); if (faceSensorProperties.isEmpty()) { @@ -454,7 +453,7 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } final int sensorId = faceSensorProperties.get(0).sensorId; - generateChallenge(sensorId, callback); + generateChallenge(sensorId, userId, callback); } /** @@ -1108,14 +1107,16 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan } /** - * Callback structure provided to {@link #generateChallenge(int, GenerateChallengeCallback)}. + * Callback structure provided to {@link #generateChallenge(int, int, + * GenerateChallengeCallback)}. + * * @hide */ public interface GenerateChallengeCallback { /** * Invoked when a challenge has been generated. */ - void onGenerateChallengeResult(int sensorId, long challenge); + void onGenerateChallengeResult(int sensorId, int userId, long challenge); } private class OnEnrollCancelListener implements OnCancelListener { @@ -1189,7 +1190,8 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan args.recycle(); break; case MSG_CHALLENGE_GENERATED: - sendChallengeGenerated(msg.arg1 /* sensorId */, (long) msg.obj /* challenge */); + sendChallengeGenerated(msg.arg1 /* sensorId */, msg.arg2 /* userId */, + (long) msg.obj /* challenge */); break; case MSG_FACE_DETECTED: sendFaceDetected(msg.arg1 /* sensorId */, msg.arg2 /* userId */, @@ -1222,11 +1224,11 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan mGetFeatureCallback.onCompleted(success, features, featureState); } - private void sendChallengeGenerated(int sensorId, long challenge) { + private void sendChallengeGenerated(int sensorId, int userId, long challenge) { if (mGenerateChallengeCallback == null) { return; } - mGenerateChallengeCallback.onGenerateChallengeResult(sensorId, challenge); + mGenerateChallengeCallback.onGenerateChallengeResult(sensorId, userId, challenge); } private void sendFaceDetected(int sensorId, int userId, boolean isStrongBiometric) { diff --git a/core/java/android/hardware/face/FaceServiceReceiver.java b/core/java/android/hardware/face/FaceServiceReceiver.java index cf2f219dbea0b..9e7859277bd22 100644 --- a/core/java/android/hardware/face/FaceServiceReceiver.java +++ b/core/java/android/hardware/face/FaceServiceReceiver.java @@ -72,7 +72,8 @@ public class FaceServiceReceiver extends IFaceServiceReceiver.Stub { } @Override - public void onChallengeGenerated(int sensorId, long challenge) throws RemoteException { + public void onChallengeGenerated(int sensorId, int userId, long challenge) + throws RemoteException { } diff --git a/core/java/android/hardware/face/IFaceServiceReceiver.aidl b/core/java/android/hardware/face/IFaceServiceReceiver.aidl index 0a28451150dd8..c4d9bf26c3ea7 100644 --- a/core/java/android/hardware/face/IFaceServiceReceiver.aidl +++ b/core/java/android/hardware/face/IFaceServiceReceiver.aidl @@ -33,7 +33,7 @@ oneway interface IFaceServiceReceiver { void onRemoved(in Face face, int remaining); void onFeatureSet(boolean success, int feature); void onFeatureGet(boolean success, in int[] features, in boolean[] featureState); - void onChallengeGenerated(int sensorId, long challenge); + void onChallengeGenerated(int sensorId, int userId, long challenge); void onAuthenticationFrame(in FaceAuthenticationFrame frame); void onEnrollmentFrame(in FaceEnrollFrame frame); } diff --git a/core/java/android/hardware/fingerprint/FingerprintManager.java b/core/java/android/hardware/fingerprint/FingerprintManager.java index b52955d035b50..8aeb5cd8f428f 100644 --- a/core/java/android/hardware/fingerprint/FingerprintManager.java +++ b/core/java/android/hardware/fingerprint/FingerprintManager.java @@ -475,10 +475,13 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing } /** + * Callbacks for generate challenge operations. + * * @hide */ public interface GenerateChallengeCallback { - void onChallengeGenerated(int sensorId, long challenge); + /** Called when a challenged has been generated. */ + void onChallengeGenerated(int sensorId, int userId, long challenge); } /** @@ -1124,7 +1127,8 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing sendRemovedResult((Fingerprint) msg.obj, msg.arg1 /* remaining */); break; case MSG_CHALLENGE_GENERATED: - sendChallengeGenerated(msg.arg1 /* sensorId */, (long) msg.obj /* challenge */); + sendChallengeGenerated(msg.arg1 /* sensorId */, msg.arg2 /* userId */, + (long) msg.obj /* challenge */); break; case MSG_FINGERPRINT_DETECTED: sendFingerprintDetected(msg.arg1 /* sensorId */, msg.arg2 /* userId */, @@ -1233,12 +1237,12 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing } } - private void sendChallengeGenerated(int sensorId, long challenge) { + private void sendChallengeGenerated(int sensorId, int userId, long challenge) { if (mGenerateChallengeCallback == null) { Slog.e(TAG, "sendChallengeGenerated, callback null"); return; } - mGenerateChallengeCallback.onChallengeGenerated(sensorId, challenge); + mGenerateChallengeCallback.onChallengeGenerated(sensorId, userId, challenge); } private void sendFingerprintDetected(int sensorId, int userId, boolean isStrongBiometric) { @@ -1454,8 +1458,8 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing } @Override // binder call - public void onChallengeGenerated(int sensorId, long challenge) { - mHandler.obtainMessage(MSG_CHALLENGE_GENERATED, sensorId, 0, challenge) + public void onChallengeGenerated(int sensorId, int userId, long challenge) { + mHandler.obtainMessage(MSG_CHALLENGE_GENERATED, sensorId, userId, challenge) .sendToTarget(); } diff --git a/core/java/android/hardware/fingerprint/FingerprintServiceReceiver.java b/core/java/android/hardware/fingerprint/FingerprintServiceReceiver.java index 798e87beb52a7..a9779b51321bb 100644 --- a/core/java/android/hardware/fingerprint/FingerprintServiceReceiver.java +++ b/core/java/android/hardware/fingerprint/FingerprintServiceReceiver.java @@ -61,7 +61,8 @@ public class FingerprintServiceReceiver extends IFingerprintServiceReceiver.Stub } @Override - public void onChallengeGenerated(int sensorId, long challenge) throws RemoteException { + public void onChallengeGenerated(int sensorId, int userId, long challenge) + throws RemoteException { } diff --git a/core/java/android/hardware/fingerprint/IFingerprintServiceReceiver.aidl b/core/java/android/hardware/fingerprint/IFingerprintServiceReceiver.aidl index 1bd284d1ec05c..9cea1fed629d3 100644 --- a/core/java/android/hardware/fingerprint/IFingerprintServiceReceiver.aidl +++ b/core/java/android/hardware/fingerprint/IFingerprintServiceReceiver.aidl @@ -29,7 +29,7 @@ oneway interface IFingerprintServiceReceiver { void onAuthenticationFailed(); void onError(int error, int vendorCode); void onRemoved(in Fingerprint fp, int remaining); - void onChallengeGenerated(int sensorId, long challenge); + void onChallengeGenerated(int sensorId, int userId, long challenge); void onUdfpsPointerDown(int sensorId); void onUdfpsPointerUp(int sensorId); } diff --git a/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java b/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java index 9855103f6f5a6..6482a2eead42a 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java +++ b/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java @@ -223,6 +223,7 @@ public abstract class BaseClientMonitor extends LoggableMonitor + this.getClass().getSimpleName() + ", " + getProtoEnum() + ", " + getOwnerString() - + ", " + getCookie() + "}"; + + ", " + getCookie() + + ", " + getTargetUserId() + "}"; } } 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 5b5461d93791e..f1c786b4977cd 100644 --- a/services/core/java/com/android/server/biometrics/sensors/ClientMonitorCallbackConverter.java +++ b/services/core/java/com/android/server/biometrics/sensors/ClientMonitorCallbackConverter.java @@ -132,11 +132,13 @@ public class ClientMonitorCallbackConverter { } } - public void onChallengeGenerated(int sensorId, long challenge) throws RemoteException { + /** Called when a challenged has been generated. */ + public void onChallengeGenerated(int sensorId, int userId, long challenge) + throws RemoteException { if (mFaceServiceReceiver != null) { - mFaceServiceReceiver.onChallengeGenerated(sensorId, challenge); + mFaceServiceReceiver.onChallengeGenerated(sensorId, userId, challenge); } else if (mFingerprintServiceReceiver != null) { - mFingerprintServiceReceiver.onChallengeGenerated(sensorId, challenge); + mFingerprintServiceReceiver.onChallengeGenerated(sensorId, userId, 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 1fcad62e3a079..3d74f369efde4 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 HalClientMonitor { @Override public void unableToStart() { try { - getListener().onChallengeGenerated(getSensorId(), 0L); + getListener().onChallengeGenerated(getSensorId(), getTargetUserId(), 0L); } catch (RemoteException e) { Slog.e(TAG, "Unable to send error", e); } diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/BiometricTestSessionImpl.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/BiometricTestSessionImpl.java index b83fba9e6a79d..57c1c74a51a84 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/BiometricTestSessionImpl.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/BiometricTestSessionImpl.java @@ -110,7 +110,7 @@ public class BiometricTestSessionImpl extends ITestSession.Stub { } @Override - public void onChallengeGenerated(int sensorId, long challenge) { + public void onChallengeGenerated(int sensorId, int userId, long challenge) { } diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceGenerateChallengeClient.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceGenerateChallengeClient.java index 904c39922a06a..d76036bf432de 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceGenerateChallengeClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceGenerateChallengeClient.java @@ -52,7 +52,7 @@ public class FaceGenerateChallengeClient extends GenerateChallengeClient 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"); + Slog.w(TAG, + "mFaceGenerateChallengeCallback not null, previous operation may be stuck"); } final List faceSensorProperties = mFaceManager.getSensorPropertiesInternal(); @@ -246,12 +236,13 @@ public class BiometricDeferredQueue { mSpManager, sensorIds, pendingResetLockouts); for (final FaceSensorPropertiesInternal prop : faceSensorProperties) { if (prop.resetLockoutRequiresHardwareAuthToken) { - if (prop.resetLockoutRequiresChallenge) { - // 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); - } else { - for (UserAuthInfo user : pendingResetLockouts) { + for (UserAuthInfo user : pendingResetLockouts) { + if (prop.resetLockoutRequiresChallenge) { + Slog.d(TAG, "Generating challenge for sensor: " + prop.sensorId + + ", user: " + user.userId); + mFaceManager.generateChallenge(prop.sensorId, user.userId, + mFaceResetLockoutTask); + } else { Slog.d(TAG, "Resetting face lockout for sensor: " + prop.sensorId + ", user: " + user.userId); final byte[] hat = requestHatFromGatekeeperPassword(mSpManager, user, diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/hidl/FaceGenerateChallengeClientTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/hidl/FaceGenerateChallengeClientTest.java index 72eae65097cf2..55dc03595b3d9 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/hidl/FaceGenerateChallengeClientTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/face/hidl/FaceGenerateChallengeClientTest.java @@ -86,20 +86,20 @@ public class FaceGenerateChallengeClientTest { @Test public void reuseResult_whenNotReady() throws Exception { mClient.reuseResult(mOtherReceiver); - verify(mOtherReceiver, never()).onChallengeGenerated(anyInt(), anyInt()); + verify(mOtherReceiver, never()).onChallengeGenerated(anyInt(), anyInt(), anyInt()); } @Test public void reuseResult_whenReady() throws Exception { mClient.start(mMonitorCallback); mClient.reuseResult(mOtherReceiver); - verify(mOtherReceiver).onChallengeGenerated(eq(SENSOR_ID), eq(CHALLENGE)); + verify(mOtherReceiver).onChallengeGenerated(eq(SENSOR_ID), eq(USER_ID), eq(CHALLENGE)); } @Test public void reuseResult_whenReallyReady() throws Exception { mClient.reuseResult(mOtherReceiver); mClient.start(mMonitorCallback); - verify(mOtherReceiver).onChallengeGenerated(eq(SENSOR_ID), eq(CHALLENGE)); + verify(mOtherReceiver).onChallengeGenerated(eq(SENSOR_ID), eq(USER_ID), eq(CHALLENGE)); } }