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

4 files changed, 36 insertions, 69 deletions

diff --git a/authsecret/1.0/IAuthSecret.hal b/authsecret/1.0/IAuthSecret.hal
index d2cb5da54..6b573b3a6 100644
--- a/authsecret/1.0/IAuthSecret.hal
+++ b/authsecret/1.0/IAuthSecret.hal
@@ -24,25 +24,23 @@ package android.hardware.authsecret@1.0;
  */
 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 46a3ec121..f9271e961 100644
--- a/authsecret/1.0/default/AuthSecret.cpp
+++ b/authsecret/1.0/default/AuthSecret.cpp
@@ -29,16 +29,12 @@ Return<void> AuthSecret::primaryUserCredential(const hidl_vec<uint8_t>& secret)
     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 edb49b856..387fa6700 100644
--- a/authsecret/1.0/default/AuthSecret.h
+++ b/authsecret/1.0/default/AuthSecret.h
@@ -22,7 +22,6 @@ using ::android::sp;
 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 b0cbd9129..a610a755b 100644
--- a/authsecret/1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp
+++ b/authsecret/1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp
@@ -30,68 +30,42 @@ struct AuthSecretHidlTest : public ::testing::VtsHalHidlTargetTestBase {
     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);
 }