From 25fe23f55f16f252b44e79229f3f2f1f3fc37aa4 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Mon, 4 Mar 2019 15:35:40 -0800 Subject: [PATCH] Finalize IBiometricsFace@1.0 Also update VTS tests Fixes: 125936336 Fixes: 124512763 Fixes: 123262389 Test: hidl-gen -L hash -r android.hardware:hardware/interfaces android.hardware.biometrics.face@1.0 add updated hash to current.txt Test: Builds Change-Id: I7949b251064d4578a89f6da834aadbfdf7e9a94f --- biometrics/face/1.0/IBiometricsFace.hal | 93 ++++++++++++------- .../1.0/IBiometricsFaceClientCallback.hal | 2 +- biometrics/face/1.0/types.hal | 18 ++-- .../VtsHalBiometricsFaceV1_0TargetTest.cpp | 63 +++++++++---- current.txt | 6 +- 5 files changed, 118 insertions(+), 64 deletions(-) diff --git a/biometrics/face/1.0/IBiometricsFace.hal b/biometrics/face/1.0/IBiometricsFace.hal index 0499c5d0fd..cd368faddb 100644 --- a/biometrics/face/1.0/IBiometricsFace.hal +++ b/biometrics/face/1.0/IBiometricsFace.hal @@ -53,6 +53,10 @@ interface IBiometricsFace { * returning to the idle state. Calling this method with the same userId * should have no effect on the state machine. * + * Note that onLockoutChanged() MUST be invoked by the implementation in + * response to a user change in order to update the framework with the + * timeout of the new user (or 0 if the user is not locked out). + * * @param userId A non-negative user identifier that must be unique and * persistent for a given user. * @param storePath filesystem path to the template storage directory. @@ -61,19 +65,22 @@ interface IBiometricsFace { setActiveUser(int32_t userId, string storePath) generates (Status status); /** - * Begins a secure transaction request, e.g. enrollment. + * Begins a secure transaction request, e.g. enroll() or resetLockout(). * * Generates a unique and cryptographically secure random token used to * indicate the start of a secure transaction. generateChallenge() and - * revokeChallenge() specify a pin/pattern/password cleared time window where - * the secure transaction is allowed. + * revokeChallenge() specify a window where the resulting HAT that is + * generated in response to checking the user's PIN/pattern/password + * can be used to verify/perform a secure transaction. * - * generateChallenge() generates a challenge which must then be wrapped by the + * generateChallenge() generates a challenge which must then be wrapped by * gatekeeper after verifying a successful strong authentication attempt, * which generates a Hardware Authentication Token. The challenge prevents - * spoofing and replay attacks and ensures that we only update a user’s face - * template if the operation was preceded by some kind of strong credential - * confirmation (e.g. device password). + * spoofing and replay attacks and ensures that only a transaction backed + * by a user authentication (PIN/pattern/password) can proceed. + * + * The implementation should be tolerant of revokeChallenge() being invoked + * after timeout has expired. * * @param challengeTimeoutSec A timeout in seconds, after which the driver * must invalidate the challenge. This is to prevent bugs or crashes in @@ -81,35 +88,35 @@ interface IBiometricsFace { * @return result, with its "value" parameter representing a "challenge": a * unique and cryptographically secure random token. */ - @callflow(next={"enroll", "revokeChallenge", "setFeatureDisabled"}) + @callflow(next={"enroll", "revokeChallenge", "setFeature"}) generateChallenge(uint32_t challengeTimeoutSec) generates (OptionalUint64 result); /** * Enrolls a user's face. * - * Note that this interface permits implementations where multiple faces can - * be enrolled for a single user. However, allowing multiple faces to be - * enrolled can be a severe security vulnerability and hence, most - * implementations must ensure that only a single face be enrolled at a - * given time. Multi-enrollment must only be used where there is a clear - * necessity for a shared use case, e.g. TVs or cars. - * - * Note that the Hardware Authentication Token must still be valid after - * this call, and must be explicitly invalidated by a call to - * revokeChallenge(). This allows clients to immediately reattempt - * enrollment (for example, if a user wasn’t satisfied with their enrollment) - * without having to go through another strong authentication flow. + * Note that the Hardware Authentication Token must be valid for the + * duration of enrollment and thus should be explicitly invalidated by a + * call to revokeChallenge() when enrollment is complete, to reduce the + * window of opportunity to re-use the challenge and HAT. For example, + * Settings calls generateChallenge() once to allow the user to enroll one + * or more faces or toggle secure settings without having to re-enter the + * PIN/pattern/password. Once the user completes the operation, Settings + * invokes revokeChallenge() to close the transaction. If the HAT is expired, + * the implementation must invoke onError with UNABLE_TO_PROCESS. * * This method triggers the IBiometricsFaceClientCallback#onEnrollResult() * method. * * @param hat A valid Hardware Authentication Token, generated as a result * of a generateChallenge() challenge being wrapped by the gatekeeper - * after a sucessful strong authentication request. - * @param timeoutSec A timeout in seconds, after which this enrollment - * attempt is cancelled. Note that the client still needs to - * call revokeChallenge() to terminate the enrollment session. + * after a successful strong authentication request. + * @param timeoutSec A timeout in seconds, after which this enroll + * attempt is cancelled. Note that the framework can continue + * enrollment by calling this again with a valid HAT. This timeout is + * expected to be used to limit power usage if the device becomes idle + * during enrollment. The implementation is expected to send + * ERROR_TIMEOUT if this happens. * @param disabledFeatures A list of features to be disabled during * enrollment. Note that all features are enabled by default. * @return status The status of this method call. @@ -122,8 +129,8 @@ interface IBiometricsFace { * Finishes the secure transaction by invalidating the challenge generated * by generateChallenge(). * - * Clients must call this method once enrollment is complete, and the user's - * face template no longer needs to be updated. + * Clients must call this method once the secure transaction (e.g. enroll + * or setFeature) is completed. See generateChallenge(). * * @return status The status of this method call. */ @@ -131,33 +138,43 @@ interface IBiometricsFace { revokeChallenge() generates (Status status); /** - * Requires all subsequent enroll/authenticate calls to use the feature. - * This method does not affect enroll, which has its own feature list. - * * Changes the state of previous enrollment setting. Because this may * decrease security, the user must enter their password before this method * is invoked (see @param HAT). The driver must verify the HAT before - * changing any feature state. + * changing any feature state. This method must return ILLEGAL_ARGUMENT if + * the HAT or faceId is invalid. This must only be invoked after + * setActiveUser() is called. + * * Note: In some cases it may not be possible to change the state of this * flag without re-enrolling. For example, if the user didn't provide * attention during the original enrollment. This flag reflects the same * persistent state as the one passed to enroll(). * + * Note: This call may block for a short amount of time (few hundred + * milliseconds). Clients are expected to invoke this asynchronously if it + * takes much longer than the above limit. Also note that the result is + * returned solely through Status (and not onError). + * * @param feature The feature to be enabled or disabled. * @param enabled True to enable the feature, false to disable. * @param hat A valid Hardware Authentication Token, generated as a result * of getChallenge(). + * @param faceId the ID of the enrollment returned by enroll() for the + * feature to update. * @return status The status of this method call. */ - setFeature(Feature feature, bool enabled, vec hat) + setFeature(Feature feature, bool enabled, vec hat, uint32_t faceId) generates(Status status); /** - * Retrieves the current state of the feature. + * Retrieves the current state of the feature. If the faceId is invalid, + * the implementation must return ILLEGAL_ARGUMENT. * - * @return enabled True if the feature is enabled, false if disabled. + * @param faceId the ID of the enrollment returned by enroll(). + * @return result with the value set to true if the feature is enabled, + * false if disabled. */ - getFeature(Feature feature) generates (bool enabled); + getFeature(Feature feature, uint32_t faceId) generates (OptionalBool result); /** * Returns an identifier associated with the current face set. @@ -176,7 +193,7 @@ interface IBiometricsFace { getAuthenticatorId() generates (OptionalUint64 result); /** - * Cancels a pending enrollment or authentication request. + * Cancels the current enroll, authenticate, remove, or enumerate operation. * * @return status The status of this method call. */ @@ -241,8 +258,12 @@ interface IBiometricsFace { /** * Reset lockout for the current user. * + * Note: This call may block for a short amount of time (few hundred + * milliseconds). Clients are expected to invoke this asynchronously if it + * takes much longer than the above limit. + * * @param hat A valid Hardware Authentication Token, generated when the - * user authenticates with Pin/Pattern/Pass. When the Hardware + * user authenticates with PIN/pattern/pass. When the Hardware * Authentication Token is verified, lockout must be reset and * onLockoutChanged must be called with duration 0. * @return status The status of this method call. diff --git a/biometrics/face/1.0/IBiometricsFaceClientCallback.hal b/biometrics/face/1.0/IBiometricsFaceClientCallback.hal index c9dd0e2628..969bc68590 100644 --- a/biometrics/face/1.0/IBiometricsFaceClientCallback.hal +++ b/biometrics/face/1.0/IBiometricsFaceClientCallback.hal @@ -39,7 +39,7 @@ interface IBiometricsFaceClientCallback { * A callback invoked when a face has been successfully authenticated. * * @param deviceId A unique id associated with the HAL implementation - * service that processed this autentication attempt. + * service that processed this authentication attempt. * @param faceId The id of the face template that passed the authentication * challenge. * @param userId The active user id for the authenticated face. diff --git a/biometrics/face/1.0/types.hal b/biometrics/face/1.0/types.hal index b5db966b47..8c4a4e9f4e 100644 --- a/biometrics/face/1.0/types.hal +++ b/biometrics/face/1.0/types.hal @@ -82,13 +82,14 @@ enum Feature : uint32_t { enum FaceError : int32_t { /** - * A hardware error has occured that cannot be resolved. Try again later. + * A hardware error has occurred that cannot be resolved. Try again later. */ HW_UNAVAILABLE = 1, /** - * The current enroll or authenticate operation could not be completed; - * the sensor was unable to process the current image. + * The current enroll or authenticate operation could not be completed, + * e.g. the sensor was unable to process the current image or the HAT was + * invalid. */ UNABLE_TO_PROCESS = 2, @@ -97,6 +98,11 @@ enum FaceError : int32_t { * prevent programs from blocking the face HAL indefinitely. The timeout is * framework and sensor-specific, but is generally on the order of 30 * seconds. + + * The timeout is a device-specific time meant to optimize power. For + * example after 30 seconds of searching for a face it can be use to + * indicate that the implementation is no longer looking and the framework + * should restart the operation on the next user interaction. */ TIMEOUT = 3, @@ -108,8 +114,8 @@ enum FaceError : int32_t { /** * The current operation has been cancelled. This may happen if a new - * request (authenticate, remove) is initiated while an on-going operation - * is in progress, or if cancel() was called. + * request (authenticate, remove, enumerate, enroll) is initiated while + * an on-going operation is in progress, or if cancel() was called. */ CANCELED = 5, @@ -357,7 +363,7 @@ struct OptionalUint64 { /** * Result structure with an addition bool field. See documentation in - * getRequireAttention() for usage of the value. + * getFeature() for usage of the value. */ struct OptionalBool { /** diff --git a/biometrics/face/1.0/vts/functional/VtsHalBiometricsFaceV1_0TargetTest.cpp b/biometrics/face/1.0/vts/functional/VtsHalBiometricsFaceV1_0TargetTest.cpp index f496bbe642..795a1ae961 100644 --- a/biometrics/face/1.0/vts/functional/VtsHalBiometricsFaceV1_0TargetTest.cpp +++ b/biometrics/face/1.0/vts/functional/VtsHalBiometricsFaceV1_0TargetTest.cpp @@ -42,6 +42,7 @@ using android::hardware::biometrics::face::V1_0::FaceError; using android::hardware::biometrics::face::V1_0::Feature; using android::hardware::biometrics::face::V1_0::IBiometricsFace; using android::hardware::biometrics::face::V1_0::IBiometricsFaceClientCallback; +using android::hardware::biometrics::face::V1_0::OptionalBool; using android::hardware::biometrics::face::V1_0::OptionalUint64; using android::hardware::biometrics::face::V1_0::Status; @@ -167,6 +168,19 @@ class RemoveCallback : public FaceCallbackBase { std::promise promise; }; +class LockoutChangedCallback : public FaceCallbackBase { + public: + Return onLockoutChanged(uint64_t duration) override { + this->hasDuration = true; + this->duration = duration; + promise.set_value(); + return Return(); + } + bool hasDuration; + uint64_t duration; + std::promise promise; +}; + // Test environment for Face HIDL HAL. class FaceHidlEnvironment : public ::testing::VtsHalHidlTargetTestEnvBase { public: @@ -266,12 +280,8 @@ TEST_F(FaceHidlTest, SetFeatureZeroHatTest) { token[i] = 0; } - Return res = mService->setFeature(Feature::REQUIRE_DIVERSITY, false, token); - ASSERT_EQ(Status::OK, static_cast(res)); - - // At least one call to onError should occur - ASSERT_TRUE(waitForCallback(cb->promise.get_future())); - ASSERT_TRUE(cb->hasError); + Return res = mService->setFeature(Feature::REQUIRE_DIVERSITY, false, token, 0); + ASSERT_EQ(Status::ILLEGAL_ARGUMENT, static_cast(res)); } // setFeature with an invalid HAT should fail. @@ -285,24 +295,27 @@ TEST_F(FaceHidlTest, SetFeatureGarbageHatTest) { token[i] = i; } - Return res = mService->setFeature(Feature::REQUIRE_DIVERSITY, false, token); - ASSERT_EQ(Status::OK, static_cast(res)); - - // At least one call to onError should occur - ASSERT_TRUE(waitForCallback(cb->promise.get_future())); - ASSERT_TRUE(cb->hasError); + Return res = mService->setFeature(Feature::REQUIRE_DIVERSITY, false, token, 0); + ASSERT_EQ(Status::ILLEGAL_ARGUMENT, static_cast(res)); +} + +void assertGetFeatureFails(sp service, int faceId, Feature feature) { + std::promise promise; + + // Features cannot be retrieved for invalid faces. + Return res = service->getFeature(feature, faceId, [&promise](const OptionalBool& result) { + ASSERT_EQ(Status::ILLEGAL_ARGUMENT, result.status); + promise.set_value(); + }); + ASSERT_TRUE(waitForCallback(promise.get_future())); } -// getFeature by default should return true for REQUIRE_ATTENTION. TEST_F(FaceHidlTest, GetFeatureRequireAttentionTest) { - Return res = mService->getFeature(Feature::REQUIRE_ATTENTION); - ASSERT_EQ(true, static_cast(res)); + assertGetFeatureFails(mService, 0 /* faceId */, Feature::REQUIRE_ATTENTION); } -// getFeature by default should return true for REQUIRE_DIVERSITY. TEST_F(FaceHidlTest, GetFeatureRequireDiversityTest) { - Return res = mService->getFeature(Feature::REQUIRE_DIVERSITY); - ASSERT_EQ(true, static_cast(res)); + assertGetFeatureFails(mService, 0 /* faceId */, Feature::REQUIRE_DIVERSITY); } // revokeChallenge should always return within the timeout @@ -400,6 +413,20 @@ TEST_F(FaceHidlTest, CancelTest) { ASSERT_EQ(FaceError::CANCELED, cb->error); } +TEST_F(FaceHidlTest, OnLockoutChangedTest) { + sp cb = new LockoutChangedCallback(); + mService->setCallback(cb, kAssertCallbackIsSet); + + // Update active user and ensure lockout duration 0 is received + mService->setActiveUser(5, kTmpDir); + + // Make sure callback was invoked + ASSERT_TRUE(waitForCallback(cb->promise.get_future())); + + // Check that duration 0 was received + ASSERT_EQ(0, cb->duration); +} + } // anonymous namespace int main(int argc, char** argv) { diff --git a/current.txt b/current.txt index bb9b22f7e2..6a4f9cbba4 100644 --- a/current.txt +++ b/current.txt @@ -435,9 +435,9 @@ ca15a738dedc2f4981925f7d7ff29c22bc3f8a848403dcf0c592c167de09d9af android.hardwar 443659bb9e27221e5da0d16c7a0ecb2dc3a9a03acc8a0b2196b47c50735e2d2e android.hardware.audio.effect@5.0::IVirtualizerEffect 78fed26a781cdca1b3bcb37520bff705d7764ee81db9cfd37014953c7ad2596e android.hardware.audio.effect@5.0::IVisualizerEffect 6385b6accab8a544e2ee54ba7bf5aa55dff6153bcedd80fdaae16fe9e0be7050 android.hardware.audio.effect@5.0::types -2f11e4c10ebe2b600426e0695f3c720d21663501c1c9449537055f13f37600d3 android.hardware.biometrics.face@1.0::IBiometricsFace -dfb0666af59eb306c82a6f576c65a160e6829d3324211a10429fd63768df70df android.hardware.biometrics.face@1.0::IBiometricsFaceClientCallback -cc40d308f38b6a218fcf99f264ebb49544fce670a6abdf294c617357a3d83dad android.hardware.biometrics.face@1.0::types +baf5a0cbf357035394be02d87334890228338fae715f7ab06e2f0e7d05abe656 android.hardware.biometrics.face@1.0::IBiometricsFace +6cbf288d6e6a9f6f0c09d1cde66318aa5b6a8c525778a61ccaedddb090f5e9cb android.hardware.biometrics.face@1.0::IBiometricsFaceClientCallback +ef5d339413d0c94a5b58baafbe3e4bccfd9ed2e7328fbbb5c25471c79923ed2f android.hardware.biometrics.face@1.0::types ecedc58dbcdb13503c19c0ab160ac1dd0530bb1471164149282dd1463c684185 android.hardware.bluetooth.audio@2.0::IBluetoothAudioPort fb9c40e4deab40be5476477078fe3d8a4a4495fd9deef4321878d169d675c633 android.hardware.bluetooth.audio@2.0::IBluetoothAudioProvider f7431f3e3e4e3387fc6f27a6cf423eddcd824a395dc4349d302c995ab44a9895 android.hardware.bluetooth.audio@2.0::IBluetoothAudioProvidersFactory