From 1a336b0054cca6e99c5fd682d91f5bcb5dc86abc Mon Sep 17 00:00:00 2001 From: Haining Chen Date: Wed, 25 Jan 2023 11:16:34 -0800 Subject: [PATCH] Switch to use addAuthenticatorsRegisteredCallback to get sensor props Directly using getSensorPropertiesInternal to get fingerprint/face sensor props is fine but may potentially have issues of race condition or HAL taking too long to response during initialization. Switch to use addAuthenticatorsRegisteredCallback which is safer. The callback will be invoked immediately if sensor props are already available, or invoked until registration of all sensors is done. Fix: 266709333 Test: atest BinaryTransparencyServiceTest Test: Mannually set the flag to true/false via: adb shell device_config put biometrics enable_biometric_property_verification true/false Then check whether data is logged via: m statsd_testdrive && statsd_testdrive 587 Change-Id: Ief69fe426e3beb8983e64032a845b7e5e3cf6c28 --- .../server/BinaryTransparencyService.java | 61 +++++++++---- .../server/BinaryTransparencyServiceTest.java | 87 +++++++++++-------- 2 files changed, 97 insertions(+), 51 deletions(-) diff --git a/services/core/java/com/android/server/BinaryTransparencyService.java b/services/core/java/com/android/server/BinaryTransparencyService.java index 8f594a5a378a2..99d6e0cb7edf3 100644 --- a/services/core/java/com/android/server/BinaryTransparencyService.java +++ b/services/core/java/com/android/server/BinaryTransparencyService.java @@ -53,9 +53,12 @@ import android.hardware.biometrics.SensorProperties; import android.hardware.biometrics.SensorProperties.ComponentInfo; import android.hardware.face.FaceManager; import android.hardware.face.FaceSensorProperties; +import android.hardware.face.FaceSensorPropertiesInternal; +import android.hardware.face.IFaceAuthenticatorsRegisteredCallback; import android.hardware.fingerprint.FingerprintManager; import android.hardware.fingerprint.FingerprintSensorProperties; import android.hardware.fingerprint.FingerprintSensorPropertiesInternal; +import android.hardware.fingerprint.IFingerprintAuthenticatorsRegisteredCallback; import android.net.Uri; import android.os.Binder; import android.os.Build; @@ -1371,25 +1374,51 @@ public class BinaryTransparencyService extends SystemService { } if (fpManager != null) { - // Log data for each fingerprint sensor - for (FingerprintSensorPropertiesInternal propInternal : - fpManager.getSensorPropertiesInternal()) { - final FingerprintSensorProperties prop = - FingerprintSensorProperties.from(propInternal); - logBiometricProperties(prop, - FrameworkStatsLog - .BIOMETRIC_PROPERTIES_COLLECTED__MODALITY__MODALITY_FINGERPRINT, - toFingerprintSensorType(prop.getSensorType())); - } + final int fpModality = FrameworkStatsLog + .BIOMETRIC_PROPERTIES_COLLECTED__MODALITY__MODALITY_FINGERPRINT; + fpManager.addAuthenticatorsRegisteredCallback( + new IFingerprintAuthenticatorsRegisteredCallback.Stub() { + @Override + public void onAllAuthenticatorsRegistered( + List sensors) { + if (DEBUG) { + Slog.d(TAG, "Retrieve fingerprint sensor properties. " + + "sensors.size()=" + sensors.size()); + } + // Log data for each fingerprint sensor + for (FingerprintSensorPropertiesInternal propInternal : sensors) { + final FingerprintSensorProperties prop = + FingerprintSensorProperties.from(propInternal); + logBiometricProperties(prop, + fpModality, + toFingerprintSensorType(prop.getSensorType())); + } + } + }); } if (faceManager != null) { - // Log data for each face sensor - for (FaceSensorProperties prop : faceManager.getSensorProperties()) { - logBiometricProperties(prop, - FrameworkStatsLog.BIOMETRIC_PROPERTIES_COLLECTED__MODALITY__MODALITY_FACE, - toFaceSensorType(prop.getSensorType())); - } + final int faceModality = FrameworkStatsLog + .BIOMETRIC_PROPERTIES_COLLECTED__MODALITY__MODALITY_FACE; + faceManager.addAuthenticatorsRegisteredCallback( + new IFaceAuthenticatorsRegisteredCallback.Stub() { + @Override + public void onAllAuthenticatorsRegistered( + List sensors) { + if (DEBUG) { + Slog.d(TAG, "Retrieve face sensor properties. sensors.size()=" + + sensors.size()); + } + // Log data for each face sensor + for (FaceSensorPropertiesInternal propInternal : sensors) { + final FaceSensorProperties prop = + FaceSensorProperties.from(propInternal); + logBiometricProperties(prop, + faceModality, + toFaceSensorType(prop.getSensorType())); + } + } + }); } } diff --git a/services/tests/servicestests/src/com/android/server/BinaryTransparencyServiceTest.java b/services/tests/servicestests/src/com/android/server/BinaryTransparencyServiceTest.java index 0b25f38ed2822..49f27e99f5ace 100644 --- a/services/tests/servicestests/src/com/android/server/BinaryTransparencyServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/BinaryTransparencyServiceTest.java @@ -16,6 +16,8 @@ package com.android.server; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.never; @@ -33,9 +35,11 @@ import android.hardware.biometrics.SensorProperties; import android.hardware.face.FaceManager; import android.hardware.face.FaceSensorProperties; import android.hardware.face.FaceSensorPropertiesInternal; +import android.hardware.face.IFaceAuthenticatorsRegisteredCallback; import android.hardware.fingerprint.FingerprintManager; import android.hardware.fingerprint.FingerprintSensorProperties; import android.hardware.fingerprint.FingerprintSensorPropertiesInternal; +import android.hardware.fingerprint.IFingerprintAuthenticatorsRegisteredCallback; import android.os.Bundle; import android.os.RemoteException; import android.os.ResultReceiver; @@ -53,6 +57,8 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -74,6 +80,15 @@ public class BinaryTransparencyServiceTest { private FingerprintManager mFpManager; @Mock private FaceManager mFaceManager; + @Mock + private PackageManager mPackageManager; + + @Captor + private ArgumentCaptor + mFpAuthenticatorsRegisteredCaptor; + @Captor + private ArgumentCaptor + mFaceAuthenticatorsRegisteredCaptor; @Before public void setUp() { @@ -83,9 +98,6 @@ public class BinaryTransparencyServiceTest { mBinaryTransparencyService = new BinaryTransparencyService(mContext, mBiometricLogger); mTestInterface = mBinaryTransparencyService.new BinaryTransparencyServiceImpl(); mOriginalBiometricsFlags = DeviceConfig.getProperties(DeviceConfig.NAMESPACE_BIOMETRICS); - - when(mContext.getSystemService(FingerprintManager.class)).thenReturn(mFpManager); - when(mContext.getSystemService(FaceManager.class)).thenReturn(mFaceManager); } @After @@ -112,6 +124,14 @@ public class BinaryTransparencyServiceTest { args, null, new ResultReceiver(null)); } + private void prepBiometricsTesting() { + when(mContext.getPackageManager()).thenReturn(mPackageManager); + when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_FINGERPRINT)).thenReturn(true); + when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_FACE)).thenReturn(true); + when(mContext.getSystemService(FingerprintManager.class)).thenReturn(mFpManager); + when(mContext.getSystemService(FaceManager.class)).thenReturn(mFaceManager); + } + @Test public void getSignedImageInfo_preInitialize_returnsUninitializedString() { String result = mTestInterface.getSignedImageInfo(); @@ -175,25 +195,14 @@ public class BinaryTransparencyServiceTest { mBinaryTransparencyService.collectBiometricProperties(); - verify(mFpManager, never()).getSensorPropertiesInternal(); - verify(mFaceManager, never()).getSensorProperties(); + verify(mBiometricLogger, never()).logStats(anyInt(), anyInt(), anyInt(), anyInt(), + anyString(), anyString(), anyString(), anyString(), anyString()); } @Test - public void testCollectBiometricProperties_enablesFeature() { - DeviceConfig.setProperty(DeviceConfig.NAMESPACE_BIOMETRICS, - BinaryTransparencyService.KEY_ENABLE_BIOMETRIC_PROPERTY_VERIFICATION, - Boolean.TRUE.toString(), - false /* makeDefault */); - - mBinaryTransparencyService.collectBiometricProperties(); - - verify(mFpManager, times(1)).getSensorPropertiesInternal(); - verify(mFaceManager, times(1)).getSensorProperties(); - } - - @Test - public void testCollectBiometricProperties_enablesFeature_logsFingerprintProperties() { + public void testCollectBiometricProperties_enablesFeature_logsFingerprintProperties() + throws RemoteException { + prepBiometricsTesting(); DeviceConfig.setProperty(DeviceConfig.NAMESPACE_BIOMETRICS, BinaryTransparencyService.KEY_ENABLE_BIOMETRIC_PROPERTY_VERIFICATION, Boolean.TRUE.toString(), @@ -209,10 +218,13 @@ public class BinaryTransparencyServiceTest { "" /* softwareVersion */)), FingerprintSensorProperties.TYPE_REAR, true /* resetLockoutRequiresHardwareAuthToken */)); - when(mFpManager.getSensorPropertiesInternal()).thenReturn(props); mBinaryTransparencyService.collectBiometricProperties(); + verify(mFpManager).addAuthenticatorsRegisteredCallback(mFpAuthenticatorsRegisteredCaptor + .capture()); + mFpAuthenticatorsRegisteredCaptor.getValue().onAllAuthenticatorsRegistered(props); + verify(mBiometricLogger, times(1)).logStats( eq(1) /* sensorId */, eq(FrameworkStatsLog @@ -230,28 +242,33 @@ public class BinaryTransparencyServiceTest { } @Test - public void testCollectBiometricProperties_enablesFeature_logsFaceProperties() { + public void testCollectBiometricProperties_enablesFeature_logsFaceProperties() + throws RemoteException { + prepBiometricsTesting(); DeviceConfig.setProperty(DeviceConfig.NAMESPACE_BIOMETRICS, BinaryTransparencyService.KEY_ENABLE_BIOMETRIC_PROPERTY_VERIFICATION, Boolean.TRUE.toString(), false /* makeDefault */); - final List props = List.of(FaceSensorProperties.from( - new FaceSensorPropertiesInternal( - 1 /* sensorId */, - SensorProperties.STRENGTH_CONVENIENCE, - 1 /* maxEnrollmentsPerUser */, - List.of(new ComponentInfoInternal("sensor" /* componentId */, - "vendor/model/revision" /* hardwareVersion */, - "1.01" /* firmwareVersion */, "00000001" /* serialNumber */, - "" /* softwareVersion */)), - FaceSensorProperties.TYPE_RGB, - true /* supportsFaceDetection */, - true /* supportsSelfIllumination */, - true /* resetLockoutRequiresHardwareAuthToken */))); - when(mFaceManager.getSensorProperties()).thenReturn(props); + final List props = List.of( + new FaceSensorPropertiesInternal( + 1 /* sensorId */, + SensorProperties.STRENGTH_CONVENIENCE, + 1 /* maxEnrollmentsPerUser */, + List.of(new ComponentInfoInternal("sensor" /* componentId */, + "vendor/model/revision" /* hardwareVersion */, + "1.01" /* firmwareVersion */, "00000001" /* serialNumber */, + "" /* softwareVersion */)), + FaceSensorProperties.TYPE_RGB, + true /* supportsFaceDetection */, + true /* supportsSelfIllumination */, + true /* resetLockoutRequiresHardwareAuthToken */)); mBinaryTransparencyService.collectBiometricProperties(); + verify(mFaceManager).addAuthenticatorsRegisteredCallback(mFaceAuthenticatorsRegisteredCaptor + .capture()); + mFaceAuthenticatorsRegisteredCaptor.getValue().onAllAuthenticatorsRegistered(props); + verify(mBiometricLogger, times(1)).logStats( eq(1) /* sensorId */, eq(FrameworkStatsLog