diff options
8 files changed, 117 insertions, 40 deletions
diff --git a/core/java/android/hardware/biometrics/IBiometricServiceReceiverInternal.aidl b/core/java/android/hardware/biometrics/IBiometricServiceReceiverInternal.aidl index 8bcaf828be97..e7219caf6cd8 100644 --- a/core/java/android/hardware/biometrics/IBiometricServiceReceiverInternal.aidl +++ b/core/java/android/hardware/biometrics/IBiometricServiceReceiverInternal.aidl @@ -26,7 +26,9 @@ package android.hardware.biometrics; oneway interface IBiometricServiceReceiverInternal { // Notify BiometricService that authentication was successful. If user confirmation is required, // the auth token must be submitted into KeyStore. - void onAuthenticationSucceeded(boolean requireConfirmation, in byte[] token); + // TODO(b/151967372): Strength should be changed to authenticatorId + void onAuthenticationSucceeded(boolean requireConfirmation, in byte[] token, + boolean isStrongBiometric); // Notify BiometricService authentication was rejected. void onAuthenticationFailed(); // Notify BiometricService than an error has occured. Forward to the correct receiver depending diff --git a/services/core/java/com/android/server/biometrics/AuthService.java b/services/core/java/com/android/server/biometrics/AuthService.java index d45ffd9918e4..ff8e3a973641 100644 --- a/services/core/java/com/android/server/biometrics/AuthService.java +++ b/services/core/java/com/android/server/biometrics/AuthService.java @@ -202,8 +202,7 @@ public class AuthService extends SystemService { // Only allow internal clients to call canAuthenticate with a different userId. final int callingUserId = UserHandle.getCallingUserId(); - Slog.d(TAG, "canAuthenticate, userId: " + userId + ", callingUserId: " + callingUserId - + ", authenticators: " + authenticators); + if (userId != callingUserId) { checkInternalPermission(); } else { @@ -212,8 +211,14 @@ public class AuthService extends SystemService { final long identity = Binder.clearCallingIdentity(); try { - return mBiometricService.canAuthenticate( + final int result = mBiometricService.canAuthenticate( opPackageName, userId, callingUserId, authenticators); + Slog.d(TAG, "canAuthenticate" + + ", userId: " + userId + + ", callingUserId: " + callingUserId + + ", authenticators: " + authenticators + + ", result: " + result); + return result; } finally { Binder.restoreCallingIdentity(identity); } diff --git a/services/core/java/com/android/server/biometrics/AuthenticationClient.java b/services/core/java/com/android/server/biometrics/AuthenticationClient.java index 766e5c4d638f..5d334c22f2db 100644 --- a/services/core/java/com/android/server/biometrics/AuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/AuthenticationClient.java @@ -66,6 +66,8 @@ public abstract class AuthenticationClient extends ClientMonitor { public abstract boolean wasUserDetected(); + public abstract boolean isStrongBiometric(); + public AuthenticationClient(Context context, Constants constants, BiometricServiceBase.DaemonWrapper daemon, long halDeviceId, IBinder token, BiometricServiceBase.ServiceListener listener, int targetUserId, int groupId, long opId, @@ -167,9 +169,15 @@ public abstract class AuthenticationClient extends ClientMonitor { } if (isBiometricPrompt() && listener != null) { // BiometricService will add the token to keystore - listener.onAuthenticationSucceededInternal(mRequireConfirmation, byteToken); + listener.onAuthenticationSucceededInternal(mRequireConfirmation, byteToken, + isStrongBiometric()); } else if (!isBiometricPrompt() && listener != null) { - KeyStore.getInstance().addAuthToken(byteToken); + if (isStrongBiometric()) { + KeyStore.getInstance().addAuthToken(byteToken); + } else { + Slog.d(getLogTag(), "Skipping addAuthToken"); + } + try { // Explicitly have if/else here to make it super obvious in case the code is // touched in the future. diff --git a/services/core/java/com/android/server/biometrics/BiometricService.java b/services/core/java/com/android/server/biometrics/BiometricService.java index e7c09baf3aeb..233416d663d9 100644 --- a/services/core/java/com/android/server/biometrics/BiometricService.java +++ b/services/core/java/com/android/server/biometrics/BiometricService.java @@ -266,7 +266,8 @@ public class BiometricService extends SystemService { SomeArgs args = (SomeArgs) msg.obj; handleAuthenticationSucceeded( (boolean) args.arg1 /* requireConfirmation */, - (byte[]) args.arg2 /* token */); + (byte[]) args.arg2 /* token */, + (boolean) args.arg3 /* isStrongBiometric */); args.recycle(); break; } @@ -568,10 +569,12 @@ public class BiometricService extends SystemService { final IBiometricServiceReceiverInternal mInternalReceiver = new IBiometricServiceReceiverInternal.Stub() { @Override - public void onAuthenticationSucceeded(boolean requireConfirmation, byte[] token) { + public void onAuthenticationSucceeded(boolean requireConfirmation, byte[] token, + boolean isStrongBiometric) { SomeArgs args = SomeArgs.obtain(); args.arg1 = requireConfirmation; args.arg2 = token; + args.arg3 = isStrongBiometric; mHandler.obtainMessage(MSG_ON_AUTHENTICATION_SUCCEEDED, args).sendToTarget(); } @@ -761,8 +764,13 @@ public class BiometricService extends SystemService { + " config_biometric_sensors?"); } + // Note that we allow BIOMETRIC_CONVENIENCE to register because BiometricService + // also does / will do other things such as keep track of lock screen timeout, etc. + // Just because a biometric is registered does not mean it can participate in + // the android.hardware.biometrics APIs. if (strength != Authenticators.BIOMETRIC_STRONG - && strength != Authenticators.BIOMETRIC_WEAK) { + && strength != Authenticators.BIOMETRIC_WEAK + && strength != Authenticators.BIOMETRIC_CONVENIENCE) { throw new IllegalStateException("Unsupported strength"); } @@ -1189,8 +1197,10 @@ public class BiometricService extends SystemService { BiometricConstants.BIOMETRIC_ERROR_NO_DEVICE_CREDENTIAL); } } else { + // This should not be possible via the public API surface and is here mainly for + // "correctness". An exception should have been thrown before getting here. Slog.e(TAG, "No authenticators requested"); - return new Pair<>(TYPE_NONE, BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE); + return new Pair<>(TYPE_NONE, BiometricConstants.BIOMETRIC_ERROR_HW_NOT_PRESENT); } } @@ -1286,7 +1296,8 @@ public class BiometricService extends SystemService { return modality; } - private void handleAuthenticationSucceeded(boolean requireConfirmation, byte[] token) { + private void handleAuthenticationSucceeded(boolean requireConfirmation, byte[] token, + boolean isStrongBiometric) { try { // Should never happen, log this to catch bad HAL behavior (e.g. auth succeeded // after user dismissed/canceled dialog). @@ -1295,9 +1306,16 @@ public class BiometricService extends SystemService { return; } - // Store the auth token and submit it to keystore after the dialog is confirmed / - // animating away. - mCurrentAuthSession.mTokenEscrow = token; + if (isStrongBiometric) { + // Store the auth token and submit it to keystore after the dialog is confirmed / + // animating away. + mCurrentAuthSession.mTokenEscrow = token; + } else { + if (token != null) { + Slog.w(TAG, "Dropping authToken for non-strong biometric"); + } + } + if (!requireConfirmation) { mCurrentAuthSession.mState = STATE_AUTHENTICATED_PENDING_SYSUI; } else { diff --git a/services/core/java/com/android/server/biometrics/BiometricServiceBase.java b/services/core/java/com/android/server/biometrics/BiometricServiceBase.java index ebd407d0e981..45b93834c1e2 100644 --- a/services/core/java/com/android/server/biometrics/BiometricServiceBase.java +++ b/services/core/java/com/android/server/biometrics/BiometricServiceBase.java @@ -413,8 +413,8 @@ public abstract class BiometricServiceBase extends SystemService throw new UnsupportedOperationException("Stub!"); } - default void onAuthenticationSucceededInternal(boolean requireConfirmation, byte[] token) - throws RemoteException { + default void onAuthenticationSucceededInternal(boolean requireConfirmation, byte[] token, + boolean isStrongBiometric) throws RemoteException { throw new UnsupportedOperationException("Stub!"); } @@ -451,10 +451,11 @@ public abstract class BiometricServiceBase extends SystemService } @Override - public void onAuthenticationSucceededInternal(boolean requireConfirmation, byte[] token) - throws RemoteException { + public void onAuthenticationSucceededInternal(boolean requireConfirmation, byte[] token, + boolean isStrongBiometric) throws RemoteException { if (getWrapperReceiver() != null) { - getWrapperReceiver().onAuthenticationSucceeded(requireConfirmation, token); + getWrapperReceiver().onAuthenticationSucceeded(requireConfirmation, token, + isStrongBiometric); } } diff --git a/services/core/java/com/android/server/biometrics/face/FaceService.java b/services/core/java/com/android/server/biometrics/face/FaceService.java index fd54129a3263..3ecf87c6860f 100644 --- a/services/core/java/com/android/server/biometrics/face/FaceService.java +++ b/services/core/java/com/android/server/biometrics/face/FaceService.java @@ -236,6 +236,11 @@ public class FaceService extends BiometricServiceBase { } @Override + public boolean isStrongBiometric() { + return FaceService.this.isStrongBiometric(); + } + + @Override public boolean onAuthenticated(BiometricAuthenticator.Identifier identifier, boolean authenticated, ArrayList<Byte> token) { final boolean result = super.onAuthenticated(identifier, authenticated, token); diff --git a/services/core/java/com/android/server/biometrics/fingerprint/FingerprintService.java b/services/core/java/com/android/server/biometrics/fingerprint/FingerprintService.java index acb1a2ffb754..8520f5aa0632 100644 --- a/services/core/java/com/android/server/biometrics/fingerprint/FingerprintService.java +++ b/services/core/java/com/android/server/biometrics/fingerprint/FingerprintService.java @@ -161,6 +161,11 @@ public class FingerprintService extends BiometricServiceBase { } @Override + public boolean isStrongBiometric() { + return FingerprintService.this.isStrongBiometric(); + } + + @Override public int handleFailedAttempt() { final int currentUser = ActivityManager.getCurrentUser(); mFailedAttempts.put(currentUser, mFailedAttempts.get(currentUser, 0) + 1); diff --git a/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java b/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java index 3ad905476190..d2925263125d 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java @@ -64,10 +64,13 @@ import com.android.internal.statusbar.IStatusBarService; import org.junit.Before; import org.junit.Test; +import org.mockito.AdditionalMatchers; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.Random; + @SmallTest public class BiometricServiceTest { @@ -347,9 +350,19 @@ public class BiometricServiceTest { } @Test - public void testAuthenticate_happyPathWithoutConfirmation() throws Exception { + public void testAuthenticate_happyPathWithoutConfirmation_strongBiometric() throws Exception { setupAuthForOnly(BiometricAuthenticator.TYPE_FINGERPRINT, Authenticators.BIOMETRIC_STRONG); + testAuthenticate_happyPathWithoutConfirmation(true /* isStrongBiometric */); + } + + @Test + public void testAuthenticate_happyPathWithoutConfirmation_weakBiometric() throws Exception { + setupAuthForOnly(BiometricAuthenticator.TYPE_FINGERPRINT, Authenticators.BIOMETRIC_WEAK); + testAuthenticate_happyPathWithoutConfirmation(false /* isStrongBiometric */); + } + private void testAuthenticate_happyPathWithoutConfirmation(boolean isStrongBiometric) + throws Exception { // Start testing the happy path invokeAuthenticate(mBiometricService.mImpl, mReceiver1, false /* requireConfirmation */, null /* authenticators */); @@ -397,9 +410,11 @@ public class BiometricServiceTest { anyLong() /* sessionId */); // Hardware authenticated + final byte[] HAT = generateRandomHAT(); mBiometricService.mInternalReceiver.onAuthenticationSucceeded( false /* requireConfirmation */, - new byte[69] /* HAT */); + HAT, + isStrongBiometric /* isStrongBiometric */); waitForIdle(); // Waiting for SystemUI to send dismissed callback assertEquals(mBiometricService.mCurrentAuthSession.mState, @@ -413,7 +428,11 @@ public class BiometricServiceTest { null /* credentialAttestation */); waitForIdle(); // HAT sent to keystore - verify(mBiometricService.mKeyStore).addAuthToken(any(byte[].class)); + if (isStrongBiometric) { + verify(mBiometricService.mKeyStore).addAuthToken(AdditionalMatchers.aryEq(HAT)); + } else { + verify(mBiometricService.mKeyStore, never()).addAuthToken(any(byte[].class)); + } // Send onAuthenticated to client verify(mReceiver1).onAuthenticationSucceeded( BiometricPrompt.AUTHENTICATION_RESULT_TYPE_BIOMETRIC); @@ -447,16 +466,29 @@ public class BiometricServiceTest { } @Test - public void testAuthenticate_happyPathWithConfirmation() throws Exception { + public void testAuthenticate_happyPathWithConfirmation_strongBiometric() throws Exception { setupAuthForOnly(BiometricAuthenticator.TYPE_FACE, Authenticators.BIOMETRIC_STRONG); + testAuthenticate_happyPathWithConfirmation(true /* isStrongBiometric */); + } + + @Test + public void testAuthenticate_happyPathWithConfirmation_weakBiometric() throws Exception { + setupAuthForOnly(BiometricAuthenticator.TYPE_FACE, Authenticators.BIOMETRIC_WEAK); + testAuthenticate_happyPathWithConfirmation(false /* isStrongBiometric */); + } + + private void testAuthenticate_happyPathWithConfirmation(boolean isStrongBiometric) + throws Exception { invokeAuthenticateAndStart(mBiometricService.mImpl, mReceiver1, true /* requireConfirmation */, null /* authenticators */); // Test authentication succeeded goes to PENDING_CONFIRMATION and that the HAT is not // sent to KeyStore yet + final byte[] HAT = generateRandomHAT(); mBiometricService.mInternalReceiver.onAuthenticationSucceeded( true /* requireConfirmation */, - new byte[69] /* HAT */); + HAT, + isStrongBiometric /* isStrongBiometric */); waitForIdle(); // Waiting for SystemUI to send confirmation callback assertEquals(mBiometricService.mCurrentAuthSession.mState, @@ -468,7 +500,11 @@ public class BiometricServiceTest { BiometricPrompt.DISMISSED_REASON_BIOMETRIC_CONFIRMED, null /* credentialAttestation */); waitForIdle(); - verify(mBiometricService.mKeyStore).addAuthToken(any(byte[].class)); + if (isStrongBiometric) { + verify(mBiometricService.mKeyStore).addAuthToken(AdditionalMatchers.aryEq(HAT)); + } else { + verify(mBiometricService.mKeyStore, never()).addAuthToken(any(byte[].class)); + } verify(mReceiver1).onAuthenticationSucceeded( BiometricPrompt.AUTHENTICATION_RESULT_TYPE_BIOMETRIC); } @@ -909,7 +945,8 @@ public class BiometricServiceTest { mBiometricService.mInternalReceiver.onAuthenticationSucceeded( true /* requireConfirmation */, - new byte[69] /* HAT */); + new byte[69] /* HAT */, + true /* isStrongBiometric */); mBiometricService.mInternalReceiver.onDialogDismissed( BiometricPrompt.DISMISSED_REASON_USER_CANCEL, null /* credentialAttestation */); waitForIdle(); @@ -927,6 +964,7 @@ public class BiometricServiceTest { eq(BiometricAuthenticator.TYPE_FACE), eq(BiometricConstants.BIOMETRIC_ERROR_USER_CANCELED), eq(0 /* vendorCode */)); + verify(mBiometricService.mKeyStore, never()).addAuthToken(any(byte[].class)); assertNull(mBiometricService.mCurrentAuthSession); } @@ -1238,20 +1276,6 @@ public class BiometricServiceTest { mFingerprintAuthenticator); } - @Test(expected = IllegalStateException.class) - public void testRegistrationWithUnsupportedStrength_throwsIllegalStateException() - throws Exception { - mBiometricService = new BiometricService(mContext, mInjector); - mBiometricService.onStart(); - - // Only STRONG and WEAK are supported. Let's enforce that CONVENIENCE cannot be - // registered. If there is a compelling reason, we can remove this constraint. - mBiometricService.mImpl.registerAuthenticator( - 0 /* id */, 2 /* modality */, - Authenticators.BIOMETRIC_CONVENIENCE /* strength */, - mFingerprintAuthenticator); - } - @Test(expected = IllegalArgumentException.class) public void testRegistrationWithNullAuthenticator_throwsIllegalArgumentException() throws Exception { @@ -1508,4 +1532,13 @@ public class BiometricServiceTest { private static void waitForIdle() { InstrumentationRegistry.getInstrumentation().waitForIdleSync(); } + + private byte[] generateRandomHAT() { + byte[] HAT = new byte[69]; + Random random = new Random(); + random.nextBytes(HAT); + return HAT; + } + + } |