Get rid of kDebugAllowBundledKey

Instead, use PushFilePrepare to push test apexes into /system_ext/apex.

Also changed set up logic for apex activation tests, because it relied
on the fact that test apexes were not pre-installed.

Test: atest ApexTestCases
Bug: 154908224
Change-Id: I56b0d7e3e59dcf246edd5e1fed0dd0122bd3069e

6 files changed, 28 insertions, 26 deletions

diff --git a/apexd/Android.bp b/apexd/Android.bp
index a6afb4a2..08f9300b 100644
--- a/apexd/Android.bp
+++ b/apexd/Android.bp
@@ -219,11 +219,6 @@ cc_library_static {
   export_header_lib_headers: [
     "libutils_headers",
   ],
-  product_variables: {
-    debuggable: {
-      cflags: ["-DDEBUG_ALLOW_BUNDLED_KEY"],
-    },
-  },
 }
 
 genrule {
diff --git a/apexd/AndroidTest.xml b/apexd/AndroidTest.xml
index aea1d248..085df982 100644
--- a/apexd/AndroidTest.xml
+++ b/apexd/AndroidTest.xml
@@ -28,6 +28,14 @@
          on higher levels (e.g., PackageInstaller). -->
     <target_preparer class="com.android.tradefed.targetprep.RootTargetPreparer" />
 
+    <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer">
+      <option name="cleanup" value="true" />
+      <option name="remount-system" value="true" />
+      <option name="push" value="apex.apexd_test.apex->/system_ext/apex/apex.apexd_test.apex" />
+      <option name="push" value="apex.apexd_test_different_app.apex->/system_ext/apex/apex.apexd_test_different_app.apex" />
+      <option name="push" value="apex.apexd_test_postinstall.apex->/system_ext/apex/apex.apexd_test_postinstall.apex" />
+      <option name="push" value="apex.apexd_test_preinstall.apex->/system_ext/apex/apex.apexd_test_preinstall.apex" />
+    </target_preparer>
     <test class="com.android.tradefed.testtype.GTest" >
         <!-- Note: despite how these lines read, the test will run nicely separated out
                    of a subfolder. -->
diff --git a/apexd/apex_file.cpp b/apexd/apex_file.cpp
index ba1647e1..96642215 100644
--- a/apexd/apex_file.cpp
+++ b/apexd/apex_file.cpp
@@ -51,11 +51,6 @@ namespace {
 
 constexpr const char* kImageFilename = "apex_payload.img";
 constexpr const char* kBundledPublicKeyFilename = "apex_pubkey";
-#ifdef DEBUG_ALLOW_BUNDLED_KEY
-constexpr const bool kDebugAllowBundledKey = true;
-#else
-constexpr const bool kDebugAllowBundledKey = false;
-#endif
 
 }  // namespace
 
@@ -257,15 +252,6 @@ Result<void> verifyVbMetaSignature(const ApexFile& apex, const uint8_t* data,
       return Error() << "Error verifying " << apex.GetPath() << ": "
                      << "public key doesn't match the pre-installed one";
     }
-  } else if (kDebugAllowBundledKey) {
-    // Failing to find the matching public key in the built-in partitions
-    // is a hard error for non-debuggable build. For debuggable builds,
-    // the public key bundled in the APEX itself is used as a fallback.
-    LOG(WARNING) << "Verifying " << apex.GetPath() << " with the bundled key";
-    if (!CompareKeys(pk, pk_len, apex.GetBundledPublicKey())) {
-      return Error() << "Error verifying " << apex.GetPath() << ": "
-                     << "public key doesn't match the one bundled in the APEX";
-    }
   } else {
     return public_key.error();
   }
diff --git a/apexd/apex_file_test.cpp b/apexd/apex_file_test.cpp
index 8b82b418..cd4b1751 100644
--- a/apexd/apex_file_test.cpp
+++ b/apexd/apex_file_test.cpp
@@ -80,6 +80,7 @@ TEST(ApexFileTest, GetApexManifest) {
 }
 
 TEST(ApexFileTest, VerifyApexVerity) {
+  ASSERT_RESULT_OK(collectPreinstalledData({"/system_ext/apex"}));
   const std::string filePath = testDataDir + "apex.apexd_test.apex";
   Result<ApexFile> apexFile = ApexFile::Open(filePath);
   ASSERT_RESULT_OK(apexFile);
diff --git a/apexd/apexd_verity_test.cpp b/apexd/apexd_verity_test.cpp
index ce86dabd..cf2317c8 100644
--- a/apexd/apexd_verity_test.cpp
+++ b/apexd/apexd_verity_test.cpp
@@ -25,6 +25,7 @@
 #include <gtest/gtest.h>
 
 #include "apex_file.h"
+#include "apex_preinstalled_data.h"
 #include "apexd_test_utils.h"
 #include "apexd_verity.h"
 
@@ -44,6 +45,7 @@ static std::string GetTestFile(const std::string& name) {
 }
 
 TEST(ApexdVerityTest, ReusesHashtree) {
+  ASSERT_TRUE(IsOk(collectPreinstalledData({"/system_ext/apex"})));
   TemporaryDir td;
 
   auto apex = ApexFile::Open(GetTestFile("apex.apexd_test_no_hashtree.apex"));
@@ -76,6 +78,7 @@ TEST(ApexdVerityTest, ReusesHashtree) {
 }
 
 TEST(ApexdVerityTest, RegenerateHashree) {
+  ASSERT_TRUE(IsOk(collectPreinstalledData({"/system_ext/apex"})));
   TemporaryDir td;
 
   auto apex = ApexFile::Open(GetTestFile("apex.apexd_test_no_hashtree.apex"));
diff --git a/apexd/apexservice_test.cpp b/apexd/apexservice_test.cpp
index 8bf86b1f..e76fa35c 100644
--- a/apexd/apexservice_test.cpp
+++ b/apexd/apexservice_test.cpp
@@ -136,12 +136,14 @@ class ApexServiceTest : public ::testing::Test {
 
   static bool IsSelinuxEnforced() { return 0 != security_getenforce(); }
 
-  Result<bool> IsActive(const std::string& name, int64_t version) {
+  Result<bool> IsActive(const std::string& name, int64_t version,
+                        const std::string& path) {
     std::vector<ApexInfo> list;
     android::binder::Status status = service_->getActivePackages(&list);
     if (status.isOk()) {
       for (const ApexInfo& p : list) {
-        if (p.moduleName == name && p.versionCode == version) {
+        if (p.moduleName == name && p.versionCode == version &&
+            p.modulePath == path) {
           return true;
         }
       }
@@ -957,7 +959,10 @@ class ApexServiceActivationTest : public ApexServiceTest {
 
     {
       // Check package is not active.
-      Result<bool> active = IsActive(installer_->package, installer_->version);
+      std::string path = stage_package ? installer_->test_installed_file
+                                       : installer_->test_file;
+      Result<bool> active =
+          IsActive(installer_->package, installer_->version, path);
       ASSERT_TRUE(IsOk(active));
       ASSERT_FALSE(*active);
     }
@@ -1034,7 +1039,8 @@ TEST_F(ApexServiceActivationSuccessTest, Activate) {
 
   {
     // Check package is active.
-    Result<bool> active = IsActive(installer_->package, installer_->version);
+    Result<bool> active = IsActive(installer_->package, installer_->version,
+                                   installer_->test_installed_file);
     ASSERT_TRUE(IsOk(active));
     ASSERT_TRUE(*active) << Join(GetActivePackagesStrings(), ',');
   }
@@ -1164,7 +1170,8 @@ TEST_F(ApexServiceNoHashtreeApexActivationTest, Activate) {
       << GetDebugStr(installer_.get());
   {
     // Check package is active.
-    Result<bool> active = IsActive(installer_->package, installer_->version);
+    Result<bool> active = IsActive(installer_->package, installer_->version,
+                                   installer_->test_installed_file);
     ASSERT_TRUE(IsOk(active));
     ASSERT_TRUE(*active) << Join(GetActivePackagesStrings(), ',');
   }
@@ -1192,7 +1199,8 @@ TEST_F(ApexServiceNoHashtreeApexActivationTest,
       << GetDebugStr(installer_.get());
   {
     // Check package is active.
-    Result<bool> active = IsActive(installer_->package, installer_->version);
+    Result<bool> active = IsActive(installer_->package, installer_->version,
+                                   installer_->test_installed_file);
     ASSERT_TRUE(IsOk(active));
     ASSERT_TRUE(*active) << Join(GetActivePackagesStrings(), ',');
   }
@@ -1594,7 +1602,8 @@ class ApexServicePrePostInstallTest : public ApexServiceTest {
 
     // Ensure that the package is neither active nor mounted.
     for (const InstallerUPtr& installer : installers) {
-      Result<bool> active = IsActive(installer->package, installer->version);
+      Result<bool> active = IsActive(installer->package, installer->version,
+                                     installer->test_file);
       ASSERT_TRUE(IsOk(active));
       EXPECT_FALSE(*active);
     }