From 7d07c892356684682fd4ba4c1382415b777e44dc Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Tue, 18 Feb 2020 18:18:17 -0800 Subject: [PATCH] Clean up biometric system server 1) BiometricService / AuthService always need to be started, since on Android 11 and later, the public credential auth API comes through this path. 2) Consolidate getAuthenticatorId() and expose via AuthService. This is used only by the platform during key generation. Instead of asking each individual service, AuthService will return a list of IDs for sensors which are enrolled and meet the required strength. Test: atest com.android.server.biometrics Test: fingerprint device, CtsVerifier biometric section Test: face unlock device, CtsVerifier biometric section Test: remove biometrics from device, CtsVerifier biometric section Bug: 148419762 Bug: 149795050 Change-Id: I2c5385b1cd4f343fabb0010e1fe6fb1ea8283391 --- .../android/app/SystemServiceRegistry.java | 16 ++--- .../hardware/biometrics/BiometricManager.java | 44 ++++++------- .../hardware/biometrics/BiometricPrompt.java | 35 +++++------ .../hardware/biometrics/IAuthService.aidl | 5 ++ .../biometrics/IBiometricAuthenticator.aidl | 3 + .../biometrics/IBiometricService.aidl | 5 ++ .../android/hardware/face/FaceManager.java | 19 ------ .../android/hardware/face/IFaceService.aidl | 2 +- .../fingerprint/FingerprintManager.java | 20 ------ .../fingerprint/IFingerprintService.aidl | 2 +- keystore/java/android/security/KeyStore.java | 63 ++++++------------- .../security/keystore/KeymasterUtils.java | 35 ++++------- .../server/biometrics/AuthService.java | 39 +++++++++--- .../server/biometrics/BiometricService.java | 27 ++++++++ .../biometrics/BiometricServiceBase.java | 12 +--- .../com/android/server/biometrics/Utils.java | 18 +++++- .../biometrics/face/FaceAuthenticator.java | 5 ++ .../server/biometrics/face/FaceService.java | 21 +------ .../fingerprint/FingerprintAuthenticator.java | 5 ++ .../fingerprint/FingerprintService.java | 22 ++----- .../biometrics/iris/IrisAuthenticator.java | 5 ++ .../java/com/android/server/SystemServer.java | 16 +++-- .../android/server/biometrics/UtilsTest.java | 10 +++ 23 files changed, 205 insertions(+), 224 deletions(-) diff --git a/core/java/android/app/SystemServiceRegistry.java b/core/java/android/app/SystemServiceRegistry.java index 655dd9b41c348..4dbe3cdab7ab5 100644 --- a/core/java/android/app/SystemServiceRegistry.java +++ b/core/java/android/app/SystemServiceRegistry.java @@ -916,17 +916,11 @@ public final class SystemServiceRegistry { @Override public BiometricManager createService(ContextImpl ctx) throws ServiceNotFoundException { - if (BiometricManager.hasBiometrics(ctx)) { - final IBinder binder = - ServiceManager.getServiceOrThrow(Context.AUTH_SERVICE); - final IAuthService service = - IAuthService.Stub.asInterface(binder); - return new BiometricManager(ctx.getOuterContext(), service); - } else { - // Allow access to the manager when service is null. This saves memory - // on devices without biometric hardware. - return new BiometricManager(ctx.getOuterContext(), null); - } + final IBinder binder = + ServiceManager.getServiceOrThrow(Context.AUTH_SERVICE); + final IAuthService service = + IAuthService.Stub.asInterface(binder); + return new BiometricManager(ctx.getOuterContext(), service); } }); diff --git a/core/java/android/hardware/biometrics/BiometricManager.java b/core/java/android/hardware/biometrics/BiometricManager.java index 7d66cae4c8453..8d472da1fb7cd 100644 --- a/core/java/android/hardware/biometrics/BiometricManager.java +++ b/core/java/android/hardware/biometrics/BiometricManager.java @@ -25,7 +25,6 @@ import android.annotation.RequiresPermission; import android.annotation.SystemApi; import android.annotation.SystemService; import android.content.Context; -import android.content.pm.PackageManager; import android.os.RemoteException; import android.util.Slog; @@ -160,19 +159,6 @@ public class BiometricManager { private final Context mContext; private final IAuthService mService; - private final boolean mHasHardware; - - /** - * @param context - * @return - * @hide - */ - public static boolean hasBiometrics(Context context) { - final PackageManager pm = context.getPackageManager(); - return pm.hasSystemFeature(PackageManager.FEATURE_FINGERPRINT) - || pm.hasSystemFeature(PackageManager.FEATURE_IRIS) - || pm.hasSystemFeature(PackageManager.FEATURE_FACE); - } /** * @hide @@ -182,8 +168,6 @@ public class BiometricManager { public BiometricManager(Context context, IAuthService service) { mContext = context; mService = service; - - mHasHardware = hasBiometrics(context); } /** @@ -249,12 +233,8 @@ public class BiometricManager { throw e.rethrowFromSystemServer(); } } else { - if (!mHasHardware) { - return BIOMETRIC_ERROR_NO_HARDWARE; - } else { - Slog.w(TAG, "hasEnrolledBiometrics(): Service not connected"); - return BIOMETRIC_ERROR_HW_UNAVAILABLE; - } + Slog.w(TAG, "hasEnrolledBiometrics(): Service not connected"); + return BIOMETRIC_ERROR_HW_UNAVAILABLE; } } @@ -331,5 +311,25 @@ public class BiometricManager { } } + /** + * 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 + * in Keystore land as SIDs, and are used during key generation. + * @hide + */ + @RequiresPermission(USE_BIOMETRIC_INTERNAL) + public long[] getAuthenticatorIds() { + if (mService != null) { + try { + return mService.getAuthenticatorIds(); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } else { + Slog.w(TAG, "getAuthenticatorIds(): Service not connected"); + return new long[0]; + } + } + } diff --git a/core/java/android/hardware/biometrics/BiometricPrompt.java b/core/java/android/hardware/biometrics/BiometricPrompt.java index ac36188ee61aa..a3aa258fec351 100644 --- a/core/java/android/hardware/biometrics/BiometricPrompt.java +++ b/core/java/android/hardware/biometrics/BiometricPrompt.java @@ -898,29 +898,24 @@ public class BiometricPrompt implements BiometricAuthenticator, BiometricConstan mExecutor = executor; mAuthenticationCallback = callback; final long sessionId = crypto != null ? crypto.getOpId() : 0; - if (BiometricManager.hasBiometrics(mContext)) { - final Bundle bundle; - if (crypto != null) { - // Allowed authenticators should default to BIOMETRIC_STRONG for crypto auth. - // Note that we use a new bundle here so as to not overwrite the application's - // preference, since it is possible that the same prompt configuration be used - // without a crypto object later. - bundle = new Bundle(mBundle); - bundle.putInt(KEY_AUTHENTICATORS_ALLOWED, - mBundle.getInt(KEY_AUTHENTICATORS_ALLOWED, - Authenticators.BIOMETRIC_STRONG)); - } else { - bundle = mBundle; - } - mService.authenticate(mToken, sessionId, userId, mBiometricServiceReceiver, - mContext.getOpPackageName(), bundle); + final Bundle bundle; + if (crypto != null) { + // Allowed authenticators should default to BIOMETRIC_STRONG for crypto auth. + // Note that we use a new bundle here so as to not overwrite the application's + // preference, since it is possible that the same prompt configuration be used + // without a crypto object later. + bundle = new Bundle(mBundle); + bundle.putInt(KEY_AUTHENTICATORS_ALLOWED, + mBundle.getInt(KEY_AUTHENTICATORS_ALLOWED, + Authenticators.BIOMETRIC_STRONG)); } else { - mExecutor.execute(() -> { - callback.onAuthenticationError(BiometricPrompt.BIOMETRIC_ERROR_HW_NOT_PRESENT, - mContext.getString(R.string.biometric_error_hw_unavailable)); - }); + bundle = mBundle; } + + mService.authenticate(mToken, sessionId, userId, mBiometricServiceReceiver, + mContext.getOpPackageName(), bundle); + } catch (RemoteException e) { Log.e(TAG, "Remote exception while authenticating", e); mExecutor.execute(() -> { diff --git a/core/java/android/hardware/biometrics/IAuthService.aidl b/core/java/android/hardware/biometrics/IAuthService.aidl index 2bf681f51410c..a6f6c1ea02933 100644 --- a/core/java/android/hardware/biometrics/IAuthService.aidl +++ b/core/java/android/hardware/biometrics/IAuthService.aidl @@ -51,4 +51,9 @@ interface IAuthService { // Reset the lockout when user authenticates with strong auth (e.g. PIN, pattern or password) void resetLockout(in byte [] token); + + // 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. + long[] getAuthenticatorIds(); } diff --git a/core/java/android/hardware/biometrics/IBiometricAuthenticator.aidl b/core/java/android/hardware/biometrics/IBiometricAuthenticator.aidl index 987d197996447..b4ebed7044417 100644 --- a/core/java/android/hardware/biometrics/IBiometricAuthenticator.aidl +++ b/core/java/android/hardware/biometrics/IBiometricAuthenticator.aidl @@ -55,4 +55,7 @@ interface IBiometricAuthenticator { // Explicitly set the active user (for enrolling work profile) void setActiveUser(int uid); + + // Gets the authenticator ID representing the current set of enrolled templates + long getAuthenticatorId(); } diff --git a/core/java/android/hardware/biometrics/IBiometricService.aidl b/core/java/android/hardware/biometrics/IBiometricService.aidl index 02e0a95a4c811..10295db38120f 100644 --- a/core/java/android/hardware/biometrics/IBiometricService.aidl +++ b/core/java/android/hardware/biometrics/IBiometricService.aidl @@ -61,4 +61,9 @@ interface IBiometricService { // Reset the lockout when user authenticates with strong auth (e.g. PIN, pattern or password) void resetLockout(in byte [] token); + + // 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. + long[] getAuthenticatorIds(); } diff --git a/core/java/android/hardware/face/FaceManager.java b/core/java/android/hardware/face/FaceManager.java index 3a0660db2b05b..cde6f4b1dbf00 100644 --- a/core/java/android/hardware/face/FaceManager.java +++ b/core/java/android/hardware/face/FaceManager.java @@ -584,25 +584,6 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan return false; } - /** - * Retrieves the authenticator token for binding keys to the lifecycle - * of the calling user's face. Used only by internal clients. - * - * @hide - */ - public long getAuthenticatorId() { - if (mService != null) { - try { - return mService.getAuthenticatorId(mContext.getOpPackageName()); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } - } else { - Log.w(TAG, "getAuthenticatorId(): Service not connected!"); - } - return 0; - } - /** * @hide */ diff --git a/core/java/android/hardware/face/IFaceService.aidl b/core/java/android/hardware/face/IFaceService.aidl index 8ba24735c039c..fa6ec6f505742 100644 --- a/core/java/android/hardware/face/IFaceService.aidl +++ b/core/java/android/hardware/face/IFaceService.aidl @@ -90,7 +90,7 @@ interface IFaceService { // long getHardwareDevice(int i); // Gets the authenticator ID for face - long getAuthenticatorId(String opPackageName); + long getAuthenticatorId(); // Reset the lockout when user authenticates with strong auth (e.g. PIN, pattern or password) void resetLockout(in byte [] token); diff --git a/core/java/android/hardware/fingerprint/FingerprintManager.java b/core/java/android/hardware/fingerprint/FingerprintManager.java index f301a5cddc9c0..1125e8b7d1f0f 100644 --- a/core/java/android/hardware/fingerprint/FingerprintManager.java +++ b/core/java/android/hardware/fingerprint/FingerprintManager.java @@ -756,26 +756,6 @@ public class FingerprintManager implements BiometricAuthenticator, BiometricFing return false; } - /** - * Retrieves the authenticator token for binding keys to the lifecycle - * of the calling user's fingerprints. Used only by internal clients. - * - * @hide - */ - @UnsupportedAppUsage - public long getAuthenticatorId() { - if (mService != null) { - try { - return mService.getAuthenticatorId(mContext.getOpPackageName()); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } - } else { - Slog.w(TAG, "getAuthenticatorId(): Service not connected!"); - } - return 0; - } - /** * @hide */ diff --git a/core/java/android/hardware/fingerprint/IFingerprintService.aidl b/core/java/android/hardware/fingerprint/IFingerprintService.aidl index f2ffd08d5bc8e..aff8d6f5f4058 100644 --- a/core/java/android/hardware/fingerprint/IFingerprintService.aidl +++ b/core/java/android/hardware/fingerprint/IFingerprintService.aidl @@ -91,7 +91,7 @@ interface IFingerprintService { // long getHardwareDevice(int i); // Gets the authenticator ID for fingerprint - long getAuthenticatorId(String opPackageName); + long getAuthenticatorId(); // Reset the timeout when user authenticates with strong auth (e.g. PIN, pattern or password) void resetTimeout(in byte [] cryptoToken); diff --git a/keystore/java/android/security/KeyStore.java b/keystore/java/android/security/KeyStore.java index dc57f55bb4af8..9d0fe11be46b6 100644 --- a/keystore/java/android/security/KeyStore.java +++ b/keystore/java/android/security/KeyStore.java @@ -21,9 +21,7 @@ import android.app.Application; import android.app.KeyguardManager; import android.compat.annotation.UnsupportedAppUsage; import android.content.Context; -import android.content.pm.PackageManager; -import android.hardware.face.FaceManager; -import android.hardware.fingerprint.FingerprintManager; +import android.hardware.biometrics.BiometricManager; import android.os.Binder; import android.os.Build; import android.os.IBinder; @@ -1348,19 +1346,26 @@ public class KeyStore { return new UserNotAuthenticatedException(); } - final long fingerprintOnlySid = getFingerprintOnlySid(); - if ((fingerprintOnlySid != 0) - && (keySids.contains(KeymasterArguments.toUint64(fingerprintOnlySid)))) { - // One of the key's SIDs is the current fingerprint SID -- user can be - // authenticated against that SID. - return new UserNotAuthenticatedException(); + final BiometricManager bm = mContext.getSystemService(BiometricManager.class); + long[] biometricSids = bm.getAuthenticatorIds(); + + // The key must contain every biometric SID. This is because the current API surface + // treats all biometrics (capable of keystore integration) equally. e.g. if the + // device has multiple keystore-capable sensors, and one of the sensor's SIDs + // changed, 1) there is no way for a developer to specify authentication with a + // specific sensor (the one that hasn't changed), and 2) currently the only + // signal to developers is the UserNotAuthenticatedException, which doesn't + // indicate a specific sensor. + boolean canUnlockViaBiometrics = true; + for (long sid : biometricSids) { + if (!keySids.contains(KeymasterArguments.toUint64(sid))) { + canUnlockViaBiometrics = false; + break; + } } - final long faceOnlySid = getFaceOnlySid(); - if ((faceOnlySid != 0) - && (keySids.contains(KeymasterArguments.toUint64(faceOnlySid)))) { - // One of the key's SIDs is the current face SID -- user can be - // authenticated against that SID. + if (canUnlockViaBiometrics) { + // All of the biometric SIDs are contained in the key's SIDs. return new UserNotAuthenticatedException(); } @@ -1374,36 +1379,6 @@ public class KeyStore { } } - private long getFaceOnlySid() { - final PackageManager packageManager = mContext.getPackageManager(); - if (!packageManager.hasSystemFeature(PackageManager.FEATURE_FACE)) { - return 0; - } - FaceManager faceManager = mContext.getSystemService(FaceManager.class); - if (faceManager == null) { - return 0; - } - - // TODO: Restore USE_BIOMETRIC or USE_BIOMETRIC_INTERNAL permission check in - // FaceManager.getAuthenticatorId once the ID is no longer needed here. - return faceManager.getAuthenticatorId(); - } - - private long getFingerprintOnlySid() { - final PackageManager packageManager = mContext.getPackageManager(); - if (!packageManager.hasSystemFeature(PackageManager.FEATURE_FINGERPRINT)) { - return 0; - } - FingerprintManager fingerprintManager = mContext.getSystemService(FingerprintManager.class); - if (fingerprintManager == null) { - return 0; - } - - // TODO: Restore USE_FINGERPRINT permission check in - // FingerprintManager.getAuthenticatorId once the ID is no longer needed here. - return fingerprintManager.getAuthenticatorId(); - } - /** * Returns an {@link InvalidKeyException} corresponding to the provided keystore/keymaster error * code. diff --git a/keystore/java/android/security/keystore/KeymasterUtils.java b/keystore/java/android/security/keystore/KeymasterUtils.java index 37b1f23ba55a1..4ead253f3eea5 100644 --- a/keystore/java/android/security/keystore/KeymasterUtils.java +++ b/keystore/java/android/security/keystore/KeymasterUtils.java @@ -16,9 +16,7 @@ package android.security.keystore; -import android.content.pm.PackageManager; -import android.hardware.face.FaceManager; -import android.hardware.fingerprint.FingerprintManager; +import android.hardware.biometrics.BiometricManager; import android.security.GateKeeper; import android.security.KeyStore; import android.security.keymaster.KeymasterArguments; @@ -115,28 +113,16 @@ public abstract class KeymasterUtils { } if (spec.getUserAuthenticationValidityDurationSeconds() == 0) { - PackageManager pm = KeyStore.getApplicationContext().getPackageManager(); - // Every use of this key needs to be authorized by the user. This currently means - // fingerprint or face auth. - FingerprintManager fingerprintManager = null; - FaceManager faceManager = null; + // Every use of this key needs to be authorized by the user. + final BiometricManager bm = KeyStore.getApplicationContext() + .getSystemService(BiometricManager.class); - if (pm.hasSystemFeature(PackageManager.FEATURE_FINGERPRINT)) { - fingerprintManager = KeyStore.getApplicationContext() - .getSystemService(FingerprintManager.class); - } - if (pm.hasSystemFeature(PackageManager.FEATURE_FACE)) { - faceManager = KeyStore.getApplicationContext().getSystemService(FaceManager.class); - } + // TODO: Restore permission check in getAuthenticatorIds once the ID is no longer + // needed here. - // TODO: Restore USE_FINGERPRINT permission check in - // FingerprintManager.getAuthenticatorId once the ID is no longer needed here. - final long fingerprintOnlySid = - (fingerprintManager != null) ? fingerprintManager.getAuthenticatorId() : 0; - final long faceOnlySid = - (faceManager != null) ? faceManager.getAuthenticatorId() : 0; + final long[] biometricSids = bm.getAuthenticatorIds(); - if (fingerprintOnlySid == 0 && faceOnlySid == 0) { + if (biometricSids.length == 0) { throw new IllegalStateException( "At least one biometric must be enrolled to create keys requiring user" + " authentication for every use"); @@ -148,8 +134,9 @@ public abstract class KeymasterUtils { } else if (spec.isInvalidatedByBiometricEnrollment()) { // The biometric-only SIDs will change on biometric enrollment or removal of all // enrolled templates, invalidating the key. - sids.add(fingerprintOnlySid); - sids.add(faceOnlySid); + for (long sid : biometricSids) { + sids.add(sid); + } } else { // The root SID will *not* change on fingerprint enrollment, or removal of all // enrolled fingerprints, allowing the key to remain valid. diff --git a/services/core/java/com/android/server/biometrics/AuthService.java b/services/core/java/com/android/server/biometrics/AuthService.java index 204f072a72fc6..087a18f44ff3a 100644 --- a/services/core/java/com/android/server/biometrics/AuthService.java +++ b/services/core/java/com/android/server/biometrics/AuthService.java @@ -265,6 +265,32 @@ public class AuthService extends SystemService { Binder.restoreCallingIdentity(identity); } } + + @Override + public long[] getAuthenticatorIds() throws RemoteException { + // In this method, we're not checking whether the caller is permitted to use face + // API because current authenticator ID is leaked (in a more contrived way) via Android + // Keystore (android.security.keystore package): the user of that API can create a key + // which requires face authentication for its use, and then query the key's + // characteristics (hidden API) which returns, among other things, face + // authenticator ID which was active at key creation time. + // + // Reason: The part of Android Keystore which runs inside an app's process invokes this + // method in certain cases. Those cases are not always where the developer demonstrates + // explicit intent to use biometric functionality. Thus, to avoiding throwing an + // unexpected SecurityException this method does not check whether its caller is + // permitted to use face API. + // + // The permission check should be restored once Android Keystore no longer invokes this + // method from inside app processes. + + final long identity = Binder.clearCallingIdentity(); + try { + return mBiometricService.getAuthenticatorIds(); + } finally { + Binder.restoreCallingIdentity(identity); + } + } } public AuthService(Context context) { @@ -305,8 +331,8 @@ public class AuthService extends SystemService { case TYPE_FINGERPRINT: final IFingerprintService fingerprintService = mInjector.getFingerprintService(); if (fingerprintService == null) { - Slog.e(TAG, "Attempting to register with null FingerprintService. Please check" - + " your device configuration."); + Slog.e(TAG, "Attempting to register with null FingerprintService." + + " Please check your device configuration."); return; } @@ -316,8 +342,8 @@ public class AuthService extends SystemService { case TYPE_FACE: final IFaceService faceService = mInjector.getFaceService(); if (faceService == null) { - Slog.e(TAG, "Attempting to register with null FaceService. Please check your" - + " device configuration."); + Slog.e(TAG, "Attempting to register with null FaceService. Please check " + + " your device configuration."); return; } @@ -327,8 +353,8 @@ public class AuthService extends SystemService { case TYPE_IRIS: final IIrisService irisService = mInjector.getIrisService(); if (irisService == null) { - Slog.e(TAG, "Attempting to register with null IrisService. Please check your" - + " device configuration."); + Slog.e(TAG, "Attempting to register with null IrisService. Please check" + + " your device configuration."); return; } @@ -344,7 +370,6 @@ public class AuthService extends SystemService { authenticator); } - private void checkInternalPermission() { getContext().enforceCallingOrSelfPermission(USE_BIOMETRIC_INTERNAL, "Must have USE_BIOMETRIC_INTERNAL permission"); diff --git a/services/core/java/com/android/server/biometrics/BiometricService.java b/services/core/java/com/android/server/biometrics/BiometricService.java index 3f6e88dfc087b..858c157d3ee18 100644 --- a/services/core/java/com/android/server/biometrics/BiometricService.java +++ b/services/core/java/com/android/server/biometrics/BiometricService.java @@ -825,6 +825,33 @@ public class BiometricService extends SystemService { Slog.e(TAG, "Remote exception", e); } } + + @Override // Binder call + public long[] getAuthenticatorIds() { + checkInternalPermission(); + + final List ids = new ArrayList<>(); + for (AuthenticatorWrapper authenticator : mAuthenticators) { + try { + final long id = authenticator.impl.getAuthenticatorId(); + if (Utils.isAtLeastStrength(authenticator.getActualStrength(), + Authenticators.BIOMETRIC_STRONG) && id != 0) { + ids.add(id); + } else { + Slog.d(TAG, "Authenticator " + authenticator + ", authenticatorID " + id + + " cannot participate in Keystore operations"); + } + } catch (RemoteException e) { + Slog.e(TAG, "RemoteException", e); + } + } + + long[] result = new long[ids.size()]; + for (int i = 0; i < ids.size(); i++) { + result[i] = ids.get(i); + } + return result; + } } private void checkInternalPermission() { diff --git a/services/core/java/com/android/server/biometrics/BiometricServiceBase.java b/services/core/java/com/android/server/biometrics/BiometricServiceBase.java index 0e709944b03f6..b7e3c8ef415ae 100644 --- a/services/core/java/com/android/server/biometrics/BiometricServiceBase.java +++ b/services/core/java/com/android/server/biometrics/BiometricServiceBase.java @@ -16,7 +16,6 @@ package com.android.server.biometrics; -import static android.Manifest.permission.USE_BIOMETRIC_INTERNAL; import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND_SERVICE; import android.app.ActivityManager; @@ -1212,16 +1211,11 @@ public abstract class BiometricServiceBase extends SystemService } /*** - * @param opPackageName the name of the calling package * @return authenticator id for the calling user */ - protected long getAuthenticatorId(String opPackageName) { - if (isKeyguard(opPackageName)) { - // If an app tells us it's keyguard, check that it actually is. - checkPermission(USE_BIOMETRIC_INTERNAL); - } - - final int userId = getUserOrWorkProfileId(opPackageName, UserHandle.getCallingUserId()); + protected long getAuthenticatorId() { + final int userId = getUserOrWorkProfileId(null /* clientPackage */, + UserHandle.getCallingUserId()); return mAuthenticatorIds.getOrDefault(userId, 0L); } diff --git a/services/core/java/com/android/server/biometrics/Utils.java b/services/core/java/com/android/server/biometrics/Utils.java index 8f3fd36f411bc..aec754048ed02 100644 --- a/services/core/java/com/android/server/biometrics/Utils.java +++ b/services/core/java/com/android/server/biometrics/Utils.java @@ -130,8 +130,24 @@ public class Utils { * @return true only if the sensor is at least as strong as the requested strength */ public static boolean isAtLeastStrength(int sensorStrength, int requestedStrength) { + // Clear out any bits that are not reserved for biometric + sensorStrength = sensorStrength & Authenticators.BIOMETRIC_MIN_STRENGTH; + // If the authenticator contains bits outside of the requested strength, it is too weak. - return (~requestedStrength & sensorStrength) == 0; + if ((sensorStrength & ~requestedStrength) != 0) { + return false; + } + + for (int i = Authenticators.BIOMETRIC_MAX_STRENGTH; + i <= requestedStrength; i = i << 1 | 1) { + if (i == sensorStrength) { + return true; + } + } + + Slog.e(BiometricService.TAG, "Unknown sensorStrength: " + sensorStrength + + ", requestedStrength: " + requestedStrength); + return false; } /** diff --git a/services/core/java/com/android/server/biometrics/face/FaceAuthenticator.java b/services/core/java/com/android/server/biometrics/face/FaceAuthenticator.java index 5df5b29d58a86..9d8fcc3421cbb 100644 --- a/services/core/java/com/android/server/biometrics/face/FaceAuthenticator.java +++ b/services/core/java/com/android/server/biometrics/face/FaceAuthenticator.java @@ -72,4 +72,9 @@ public final class FaceAuthenticator extends IBiometricAuthenticator.Stub { public void setActiveUser(int uid) throws RemoteException { mFaceService.setActiveUser(uid); } + + @Override + public long getAuthenticatorId() throws RemoteException { + return mFaceService.getAuthenticatorId(); + } } 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 31c3d4d7b24e3..9dba8ed889fda 100644 --- a/services/core/java/com/android/server/biometrics/face/FaceService.java +++ b/services/core/java/com/android/server/biometrics/face/FaceService.java @@ -611,24 +611,9 @@ public class FaceService extends BiometricServiceBase { } @Override // Binder call - public long getAuthenticatorId(String opPackageName) { - // In this method, we're not checking whether the caller is permitted to use face - // API because current authenticator ID is leaked (in a more contrived way) via Android - // Keystore (android.security.keystore package): the user of that API can create a key - // which requires face authentication for its use, and then query the key's - // characteristics (hidden API) which returns, among other things, face - // authenticator ID which was active at key creation time. - // - // Reason: The part of Android Keystore which runs inside an app's process invokes this - // method in certain cases. Those cases are not always where the developer demonstrates - // explicit intent to use face functionality. Thus, to avoiding throwing an - // unexpected SecurityException this method does not check whether its caller is - // permitted to use face API. - // - // The permission check should be restored once Android Keystore no longer invokes this - // method from inside app processes. - - return FaceService.this.getAuthenticatorId(opPackageName); + public long getAuthenticatorId() { + checkPermission(USE_BIOMETRIC_INTERNAL); + return FaceService.this.getAuthenticatorId(); } @Override // Binder call diff --git a/services/core/java/com/android/server/biometrics/fingerprint/FingerprintAuthenticator.java b/services/core/java/com/android/server/biometrics/fingerprint/FingerprintAuthenticator.java index 7a4e62e18474e..5bbeef153ea45 100644 --- a/services/core/java/com/android/server/biometrics/fingerprint/FingerprintAuthenticator.java +++ b/services/core/java/com/android/server/biometrics/fingerprint/FingerprintAuthenticator.java @@ -72,4 +72,9 @@ public final class FingerprintAuthenticator extends IBiometricAuthenticator.Stub public void setActiveUser(int uid) throws RemoteException { mFingerprintService.setActiveUser(uid); } + + @Override + public long getAuthenticatorId() throws RemoteException { + return mFingerprintService.getAuthenticatorId(); + } } 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 0a6198863b006..9bb927eff291d 100644 --- a/services/core/java/com/android/server/biometrics/fingerprint/FingerprintService.java +++ b/services/core/java/com/android/server/biometrics/fingerprint/FingerprintService.java @@ -21,6 +21,7 @@ import static android.Manifest.permission.MANAGE_BIOMETRIC; 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; import static android.hardware.biometrics.BiometricAuthenticator.TYPE_FINGERPRINT; @@ -410,24 +411,9 @@ public class FingerprintService extends BiometricServiceBase { } @Override // Binder call - public long getAuthenticatorId(String opPackageName) { - // In this method, we're not checking whether the caller is permitted to use fingerprint - // API because current authenticator ID is leaked (in a more contrived way) via Android - // Keystore (android.security.keystore package): the user of that API can create a key - // which requires fingerprint authentication for its use, and then query the key's - // characteristics (hidden API) which returns, among other things, fingerprint - // authenticator ID which was active at key creation time. - // - // Reason: The part of Android Keystore which runs inside an app's process invokes this - // method in certain cases. Those cases are not always where the developer demonstrates - // explicit intent to use fingerprint functionality. Thus, to avoiding throwing an - // unexpected SecurityException this method does not check whether its caller is - // permitted to use fingerprint API. - // - // The permission check should be restored once Android Keystore no longer invokes this - // method from inside app processes. - - return FingerprintService.super.getAuthenticatorId(opPackageName); + public long getAuthenticatorId() { + checkPermission(USE_BIOMETRIC_INTERNAL); + return FingerprintService.super.getAuthenticatorId(); } @Override // Binder call diff --git a/services/core/java/com/android/server/biometrics/iris/IrisAuthenticator.java b/services/core/java/com/android/server/biometrics/iris/IrisAuthenticator.java index c44b8e7227e3d..6789a12d065ff 100644 --- a/services/core/java/com/android/server/biometrics/iris/IrisAuthenticator.java +++ b/services/core/java/com/android/server/biometrics/iris/IrisAuthenticator.java @@ -65,4 +65,9 @@ public final class IrisAuthenticator extends IBiometricAuthenticator.Stub { @Override public void setActiveUser(int uid) throws RemoteException { } + + @Override + public long getAuthenticatorId() throws RemoteException { + return 0; + } } diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java index 569986c461867..cefdbd9401a8e 100644 --- a/services/java/com/android/server/SystemServer.java +++ b/services/java/com/android/server/SystemServer.java @@ -1906,16 +1906,14 @@ public final class SystemServer { t.traceEnd(); } - if (hasFeatureFace || hasFeatureIris || hasFeatureFingerprint) { - // Start this service after all biometric services. - t.traceBegin("StartBiometricService"); - mSystemServiceManager.startService(BiometricService.class); - t.traceEnd(); + // Start this service after all biometric services. + t.traceBegin("StartBiometricService"); + mSystemServiceManager.startService(BiometricService.class); + t.traceEnd(); - t.traceBegin("StartAuthService"); - mSystemServiceManager.startService(AuthService.class); - t.traceEnd(); - } + t.traceBegin("StartAuthService"); + mSystemServiceManager.startService(AuthService.class); + t.traceEnd(); t.traceBegin("StartBackgroundDexOptService"); diff --git a/services/tests/servicestests/src/com/android/server/biometrics/UtilsTest.java b/services/tests/servicestests/src/com/android/server/biometrics/UtilsTest.java index df242f3efc169..c2319d3c5d9ed 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/UtilsTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/UtilsTest.java @@ -201,6 +201,16 @@ public class UtilsTest { requestedStrength = Authenticators.BIOMETRIC_WEAK; assertTrue(Utils.isAtLeastStrength(sensorStrength, requestedStrength)); + + + // Test invalid inputs + + sensorStrength = Authenticators.BIOMETRIC_STRONG; + requestedStrength = Authenticators.DEVICE_CREDENTIAL; + assertFalse(Utils.isAtLeastStrength(sensorStrength, requestedStrength)); + + requestedStrength = 1 << 2; + assertFalse(Utils.isAtLeastStrength(sensorStrength, requestedStrength)); } @Test