authsecret: remove factoryReset()
Factory reset will be handled by each device from recovery or in the
bootloader in response to `fastboot -w`. The requirements of the factory
reset are made explicit. As a result, VTS tests are moving to a host
side test that can factory reset between each test case.
Also clarifies when the remaining method is called.
Test: AuthSecretHidlTest
Bug: 71527305
Change-Id: I9a29568e022eb83061d8db68e1e7971fc53bd823
diff --git a/authsecret/1.0/IAuthSecret.hal b/authsecret/1.0/IAuthSecret.hal
index d2cb5da..6b573b3 100644
--- a/authsecret/1.0/IAuthSecret.hal
+++ b/authsecret/1.0/IAuthSecret.hal
@@ -24,25 +24,23 @@
*/
interface IAuthSecret {
/**
- * When the primary user correctly enters their credential, this method is
- * passed a secret derived from that credential to prove that their
- * credential is known.
+ * When the primary user is unlocked, this method is passed a secret to
+ * prove that is has been successfully unlocked. The primary user can either
+ * be unlocked by a person entering their credential or by another party
+ * using an escrow token e.g. a device administrator.
*
* The first time this is called, the secret must be used to provision state
- * that depends on the primary user's credential. The same secret is passed
- * on each call until a factory reset after which there must be a new
- * secret.
+ * that depends on the primary user's secret. The same secret must be passed
+ * on each call until the next factory reset.
*
- * The secret must be at lesat 16 bytes.
+ * Upon factory reset, any dependence on the secret must be removed as that
+ * secret is now lost and must never be derived again. A new secret must be
+ * created for the new primary user which must be used to newly provision
+ * state the first time this method is called after factory reset.
+ *
+ * The secret must be at least 16 bytes.
*
* @param secret blob derived from the primary user's credential.
*/
primaryUserCredential(vec<uint8_t> secret);
-
- /**
- * Called from recovery during factory reset. The secret is now lost and can
- * no longer be derived. Any data linked to the secret must be destroyed and
- * any dependence on the secret must be removed.
- */
- factoryReset();
};
diff --git a/authsecret/1.0/default/AuthSecret.cpp b/authsecret/1.0/default/AuthSecret.cpp
index 46a3ec1..f9271e9 100644
--- a/authsecret/1.0/default/AuthSecret.cpp
+++ b/authsecret/1.0/default/AuthSecret.cpp
@@ -29,16 +29,12 @@
return Void();
}
-Return<void> AuthSecret::factoryReset() {
- // Clear all dependency on the secret.
- //
- // With the example of updating a security module, the stored value must be
- // cleared so that the new primary user enrolled as the approver of updates.
- //
- // This implementation does nothing as there is no dependence on the secret.
-
- return Void();
-}
+// Note: on factory reset, clear all dependency on the secret.
+//
+// With the example of updating a security module, the stored value must be
+// cleared so that the new primary user enrolled as the approver of updates.
+//
+// This implementation does nothing as there is no dependence on the secret.
} // namespace implementation
} // namespace V1_0
diff --git a/authsecret/1.0/default/AuthSecret.h b/authsecret/1.0/default/AuthSecret.h
index edb49b8..387fa67 100644
--- a/authsecret/1.0/default/AuthSecret.h
+++ b/authsecret/1.0/default/AuthSecret.h
@@ -22,7 +22,6 @@
struct AuthSecret : public IAuthSecret {
// Methods from ::android::hardware::authsecret::V1_0::IAuthSecret follow.
Return<void> primaryUserCredential(const hidl_vec<uint8_t>& secret) override;
- Return<void> factoryReset() override;
// Methods from ::android::hidl::base::V1_0::IBase follow.
};
diff --git a/authsecret/1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp b/authsecret/1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp
index b0cbd91..a610a75 100644
--- a/authsecret/1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp
+++ b/authsecret/1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp
@@ -30,68 +30,42 @@
virtual void SetUp() override {
authsecret = ::testing::VtsHalHidlTargetTestBase::getService<IAuthSecret>();
ASSERT_NE(authsecret, nullptr);
- authsecret->factoryReset();
+
+ // All tests must enroll the correct secret first as this cannot be changed
+ // without a factory reset and the order of tests could change.
+ authsecret->primaryUserCredential(CORRECT_SECRET);
}
sp<IAuthSecret> authsecret;
+ hidl_vec<uint8_t> CORRECT_SECRET{61, 93, 124, 240, 5, 0, 7, 201, 9, 129, 11, 12, 0, 14, 0, 16};
+ hidl_vec<uint8_t> WRONG_SECRET{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
};
/* Provision the primary user with a secret. */
TEST_F(AuthSecretHidlTest, provisionPrimaryUserCredential) {
- hidl_vec<uint8_t> secret{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
- authsecret->primaryUserCredential(secret);
-}
-
-/* Provision the primary user with a large secret. */
-TEST_F(AuthSecretHidlTest, provisionPrimaryUserCredentialWithLargeSecret) {
- hidl_vec<uint8_t> secret{89, 233, 52, 29, 130, 210, 229, 170, 124, 102, 56, 238, 198,
- 199, 246, 152, 185, 123, 155, 215, 29, 252, 30, 70, 118, 29,
- 149, 36, 222, 203, 163, 7, 72, 56, 247, 19, 198, 76, 71,
- 37, 120, 201, 220, 70, 150, 18, 23, 22, 236, 57, 184, 86,
- 190, 122, 210, 207, 74, 51, 222, 157, 74, 196, 86, 208};
- authsecret->primaryUserCredential(secret);
+ // Secret provisioned by SetUp()
}
/* Provision the primary user with a secret and pass the secret again. */
TEST_F(AuthSecretHidlTest, provisionPrimaryUserCredentialAndPassAgain) {
- hidl_vec<uint8_t> secret{64, 2, 3, 0, 5, 6, 7, 172, 9, 10, 11, 255, 13, 14, 15, 83};
- authsecret->primaryUserCredential(secret);
- authsecret->primaryUserCredential(secret);
+ // Secret provisioned by SetUp()
+ authsecret->primaryUserCredential(CORRECT_SECRET);
}
/* Provision the primary user with a secret and pass the secret again repeatedly. */
TEST_F(AuthSecretHidlTest, provisionPrimaryUserCredentialAndPassAgainMultipleTimes) {
- hidl_vec<uint8_t> secret{1, 2, 34, 4, 5, 6, 7, 8, 9, 105, 11, 12, 13, 184, 15, 16};
- authsecret->primaryUserCredential(secret);
+ // Secret provisioned by SetUp()
constexpr int N = 5;
for (int i = 0; i < N; ++i) {
- authsecret->primaryUserCredential(secret);
+ authsecret->primaryUserCredential(CORRECT_SECRET);
}
}
-/* Factory reset before provisioning the primary user with a secret. */
-TEST_F(AuthSecretHidlTest, factoryResetWithoutProvisioningPrimaryUserCredential) {
- authsecret->factoryReset();
-}
-
-/* Provision the primary user with a secret then factory reset. */
-TEST_F(AuthSecretHidlTest, provisionPrimaryUserCredentialAndFactoryReset) {
- hidl_vec<uint8_t> secret{1, 24, 124, 240, 5, 6, 7, 8, 9, 13, 11, 12, 189, 14, 195, 16};
- authsecret->primaryUserCredential(secret);
- authsecret->factoryReset();
-}
-
-/* Provision the primary differently after factory reset. */
-TEST_F(AuthSecretHidlTest, provisionPrimaryUserCredentialDifferentlyAfterFactoryReset) {
- {
- hidl_vec<uint8_t> secret1{19, 0, 65, 20, 65, 12, 7, 8, 9, 13, 29, 12, 189, 32, 195, 16};
- authsecret->primaryUserCredential(secret1);
- }
-
- authsecret->factoryReset();
-
- {
- hidl_vec<uint8_t> secret2{61, 93, 124, 240, 5, 0, 7, 201, 9, 129, 11, 12, 0, 14, 0, 16};
- authsecret->primaryUserCredential(secret2);
- }
+/* Provision the primary user with a secret and then pass the wrong secret. This
+ * should never happen and is an framework bug if it does. As the secret is
+ * wrong, the HAL implementation may not be able to function correctly but it
+ * should fail gracefully. */
+TEST_F(AuthSecretHidlTest, provisionPrimaryUserCredentialAndWrongSecret) {
+ // Secret provisioned by SetUp()
+ authsecret->primaryUserCredential(WRONG_SECRET);
}