Don't call avb_property_lookup to get apex key name

Name of the public key is equal to the name of an apex module, there is
no need to call avb_property_lookup.

Test: atest CtsStagedInstallHostTestCases
Test: atest ApexTestCases
Test: atest --test-mapping system/apex/apexd:postsubmit
Bug: 146895998
Change-Id: I0be793b91ac904570145697dbf0ac540407d7195

6 files changed, 41 insertions, 26 deletions

diff --git a/apexd/Android.bp b/apexd/Android.bp
index 08f9300b..b071da56 100644
--- a/apexd/Android.bp
+++ b/apexd/Android.bp
@@ -276,6 +276,7 @@ cc_test {
     ":apex.apexd_test_preinstall",
     ":apex.apexd_test_prepostinstall.fail",
     ":apex.apexd_test_v2",
+    ":apex.corrupted_b146895998",
     ":gen_bad_apexes",
     ":gen_corrupt_apex",
     ":com.android.apex.cts.shim.v1_prebuilt",
diff --git a/apexd/apex_file.cpp b/apexd/apex_file.cpp
index 96642215..196a81ba 100644
--- a/apexd/apex_file.cpp
+++ b/apexd/apex_file.cpp
@@ -134,8 +134,6 @@ Result<ApexFile> ApexFile::Open(const std::string& path) {
 
 namespace {
 
-static constexpr const char* kApexKeyProp = "apex.key";
-
 static constexpr int kVbMetaMaxSize = 64 * 1024;
 
 std::string bytes_to_hex(const uint8_t* bytes, size_t bytes_len) {
@@ -195,24 +193,6 @@ bool CompareKeys(const uint8_t* key, size_t length,
          memcmp(&public_key_content[0], key, length) == 0;
 }
 
-Result<std::string> getPublicKeyName(const ApexFile& apex, const uint8_t* data,
-                                     size_t length) {
-  size_t keyNameLen;
-  const char* keyName = avb_property_lookup(data, length, kApexKeyProp,
-                                            strlen(kApexKeyProp), &keyNameLen);
-  if (keyName == nullptr || keyNameLen == 0) {
-    return Error() << "Cannot find prop '" << kApexKeyProp << "' from "
-                   << apex.GetPath();
-  }
-
-  if (keyName != apex.GetManifest().name()) {
-    return Error() << "Key mismatch: apex name is '"
-                   << apex.GetManifest().name() << "'"
-                   << " but key name is '" << keyName << "'";
-  }
-  return keyName;
-}
-
 Result<void> verifyVbMetaSignature(const ApexFile& apex, const uint8_t* data,
                                    size_t length) {
   const uint8_t* pk;
@@ -238,12 +218,7 @@ Result<void> verifyVbMetaSignature(const ApexFile& apex, const uint8_t* data,
       return Errorf("Unknown vmbeta_image_verify return value");
   }
 
-  Result<std::string> key_name = getPublicKeyName(apex, data, length);
-  if (!key_name.ok()) {
-    return key_name.error();
-  }
-
-  Result<const std::string> public_key = getApexKey(*key_name);
+  Result<const std::string> public_key = getApexKey(apex.GetManifest().name());
   if (public_key.ok()) {
     // TODO(b/115718846)
     // We need to decide whether we need rollback protection, and whether
diff --git a/apexd/apex_file_test.cpp b/apexd/apex_file_test.cpp
index 839cf41e..a086388c 100644
--- a/apexd/apex_file_test.cpp
+++ b/apexd/apex_file_test.cpp
@@ -130,6 +130,13 @@ TEST(ApexFileTest, GetBundledPublicKey) {
   EXPECT_EQ(keyContent, apexFile->GetBundledPublicKey());
 }
 
+TEST(ApexFileTest, CorrutedApex_b146895998) {
+  const std::string apex_path = testDataDir + "corrupted_b146895998.apex";
+  Result<ApexFile> apex = ApexFile::Open(apex_path);
+  ASSERT_RESULT_OK(apex);
+  ASSERT_FALSE(apex->VerifyApexVerity());
+}
+
 }  // namespace
 }  // namespace apex
 }  // namespace android
diff --git a/apexd/apexd_testdata/Android.bp b/apexd/apexd_testdata/Android.bp
index 2b879248..974c22d9 100644
--- a/apexd/apexd_testdata/Android.bp
+++ b/apexd/apexd_testdata/Android.bp
@@ -178,3 +178,10 @@ prebuilt_etc {
   name: "another_prebuilt_file",
   src: "another_prebuilt_file",
 }
+
+prebuilt_apex {
+  name: "apex.corrupted_b146895998",
+  src: "corrupted_b146895998.apex",
+  filename: "corrupted_b146895998.apex",
+  installable: false,
+}
diff --git a/apexd/apexd_testdata/corrupted_b146895998.apex b/apexd/apexd_testdata/corrupted_b146895998.apex
new file mode 100644
index 00000000..fc6b4747
--- /dev/null
+++ b/apexd/apexd_testdata/corrupted_b146895998.apex
diff --git a/apexd/apexservice_test.cpp b/apexd/apexservice_test.cpp
index b83c8ae0..6ea85f1a 100644
--- a/apexd/apexservice_test.cpp
+++ b/apexd/apexservice_test.cpp
@@ -2654,6 +2654,31 @@ TEST_F(ApexServiceTest, SubmitStagedSessionCorruptApexFails) {
   ASSERT_FALSE(IsOk(service_->submitStagedSession(params, &list)));
 }
 
+TEST_F(ApexServiceTest, SubmitStagedSessionCorruptApexFails_b146895998) {
+  PrepareTestApexForInstall installer(GetTestFile("corrupted_b146895998.apex"),
+                                      "/data/app-staging/session_71",
+                                      "staging_data_file");
+
+  if (!installer.Prepare()) {
+    FAIL() << GetDebugStr(&installer);
+  }
+
+  ApexInfoList list;
+  ApexSessionParams params;
+  params.sessionId = 71;
+  ASSERT_FALSE(IsOk(service_->submitStagedSession(params, &list)));
+}
+
+TEST_F(ApexServiceTest, StageCorruptApexFails_b146895998) {
+  PrepareTestApexForInstall installer(GetTestFile("corrupted_b146895998.apex"));
+
+  if (!installer.Prepare()) {
+    FAIL() << GetDebugStr(&installer);
+  }
+
+  ASSERT_FALSE(IsOk(service_->stagePackages({installer.test_file})));
+}
+
 class LogTestToLogcat : public ::testing::EmptyTestEventListener {
   void OnTestStart(const ::testing::TestInfo& test_info) override {
 #ifdef __ANDROID__