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
This commit is contained in:
Joe Bolinger
2021-05-07 17:53:09 -07:00
parent 1652749e61
commit 72c3446049
19 changed files with 64 additions and 62 deletions

View File

@@ -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<FaceSensorPropertiesInternal> 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) {

View File

@@ -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 {
}

View File

@@ -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);
}

View File

@@ -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();
}

View File

@@ -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 {
}

View File

@@ -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);
}

View File

@@ -223,6 +223,7 @@ public abstract class BaseClientMonitor extends LoggableMonitor
+ this.getClass().getSimpleName()
+ ", " + getProtoEnum()
+ ", " + getOwnerString()
+ ", " + getCookie() + "}";
+ ", " + getCookie()
+ ", " + getTargetUserId() + "}";
}
}

View File

@@ -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);
}
}

View File

@@ -40,7 +40,7 @@ public abstract class GenerateChallengeClient<T> extends HalClientMonitor<T> {
@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);
}

View File

@@ -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) {
}

View File

@@ -52,7 +52,7 @@ public class FaceGenerateChallengeClient extends GenerateChallengeClient<ISessio
void onChallengeGenerated(int sensorId, int userId, long challenge) {
try {
getListener().onChallengeGenerated(sensorId, challenge);
getListener().onChallengeGenerated(sensorId, userId, challenge);
mCallback.onClientFinished(this, true /* success */);
} catch (RemoteException e) {
Slog.e(TAG, "Unable to send challenge", e);

View File

@@ -99,7 +99,7 @@ public class BiometricTestSessionImpl extends ITestSession.Stub {
}
@Override
public void onChallengeGenerated(int sensorId, long challenge) {
public void onChallengeGenerated(int sensorId, int userId, long challenge) {
}

View File

@@ -97,7 +97,7 @@ public class FaceGenerateChallengeClient extends GenerateChallengeClient<IBiomet
@NonNull Callback ownerCallback) {
Preconditions.checkState(mChallengeResult != null, "result not available");
try {
receiver.onChallengeGenerated(getSensorId(), mChallengeResult);
receiver.onChallengeGenerated(getSensorId(), getTargetUserId(), mChallengeResult);
ownerCallback.onClientFinished(this, true /* success */);
} catch (RemoteException e) {
Slog.e(TAG, "Remote exception", e);

View File

@@ -100,7 +100,7 @@ class BiometricTestSessionImpl extends ITestSession.Stub {
}
@Override
public void onChallengeGenerated(int sensorId, long challenge) {
public void onChallengeGenerated(int sensorId, int userId, long challenge) {
}

View File

@@ -53,7 +53,7 @@ class FingerprintGenerateChallengeClient extends GenerateChallengeClient<ISessio
void onChallengeGenerated(int sensorId, int userId, long challenge) {
try {
getListener().onChallengeGenerated(sensorId, challenge);
getListener().onChallengeGenerated(sensorId, userId, challenge);
mCallback.onClientFinished(this, true /* success */);
} catch (RemoteException e) {
Slog.e(TAG, "Unable to send challenge", e);

View File

@@ -101,7 +101,7 @@ public class BiometricTestSessionImpl extends ITestSession.Stub {
}
@Override
public void onChallengeGenerated(int sensorId, long challenge) {
public void onChallengeGenerated(int sensorId, int userId, long challenge) {
}

View File

@@ -48,7 +48,7 @@ public class FingerprintGenerateChallengeClient
try {
final long challenge = getFreshDaemon().preEnroll();
try {
getListener().onChallengeGenerated(getSensorId(), challenge);
getListener().onChallengeGenerated(getSensorId(), getTargetUserId(), challenge);
mCallback.onClientFinished(this, true /* success */);
} catch (RemoteException e) {
Slog.e(TAG, "Remote exception", e);

View File

@@ -98,7 +98,7 @@ public class BiometricDeferredQueue {
}
@Override
public void onGenerateChallengeResult(int sensorId, long challenge) {
public void onGenerateChallengeResult(int sensorId, int userId, long challenge) {
if (!sensorIds.contains(sensorId)) {
Slog.e(TAG, "Unknown sensorId received: " + sensorId);
return;
@@ -116,10 +116,6 @@ public class BiometricDeferredQueue {
}
sensorIds.remove(sensorId);
// Challenge is only required for IBiometricsFace@1.0 (and not IFace AIDL). The
// IBiometricsFace@1.0 HAL does not require userId to revokeChallenge, so passing
// in 0 is OK.
final int userId = 0;
faceManager.revokeChallenge(sensorId, userId, challenge);
if (sensorIds.isEmpty()) {
@@ -222,18 +218,12 @@ public class BiometricDeferredQueue {
}
}
/**
* 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.
*/
private void processPendingLockoutsForFace(List<UserAuthInfo> 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<FaceSensorPropertiesInternal> 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,

View File

@@ -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));
}
}