Make ApexPreinstalledData a singleton class
Storing state in a global variable inside a namespace is prone to
errors, and doesn't provide a clean api. Given that there are several
different entities that would like/already are to mount apexes, it's
time to add a cleaner api for them to use.
ApexPreinstalledData is now a non-copyable and non-moveable singletone,
with some documentation on how it's expected to be used. This is a first
cl in a sequence that tries to do a small cleanup of apexd codebase. :)
Test: presubmit
Test: atest ApexTestCases
Bug: 165948777
Change-Id: Icb26e4a7f6ed08c3702223a173ccfc399069b5d5
Merged-In: Icb26e4a7f6ed08c3702223a173ccfc399069b5d5
(cherry picked from commit 645d612e4c6fd2d3ac6e1acb6b71713cdaceb5f2)
diff --git a/apexd/Android.bp b/apexd/Android.bp
index a966790..4f0d765 100644
--- a/apexd/Android.bp
+++ b/apexd/Android.bp
@@ -324,6 +324,22 @@
"| cut -c 3-| tee $(out)"
}
+genrule {
+ // Generates an apex which has same module name as apex.apexd_test.apex, but
+ // is actually signed with a different key.
+ name: "gen_key_mismatch_apex",
+ out: ["apex.apexd_test_different_key.apex"],
+ srcs: [":apex.apexd_test_no_inst_key"],
+ tools: ["soong_zip", "zipalign", "conv_apex_manifest"],
+ cmd: "unzip -q $(in) -d $(genDir) && " +
+ "$(location conv_apex_manifest) setprop name com.android.apex.test_package $(genDir)/apex_manifest.pb && " +
+ "$(location soong_zip) -d -C $(genDir) -D $(genDir) " +
+ "-s apex_manifest.pb -s apex_payload.img -s apex_pubkey " +
+ "-o $(genDir)/unaligned.apex && " +
+ "$(location zipalign) -f 4096 $(genDir)/unaligned.apex " +
+ "$(genDir)/apex.apexd_test_different_key.apex"
+}
+
cc_test {
name: "ApexTestCases",
defaults: [
@@ -352,6 +368,7 @@
":apex.apexd_test_prepostinstall.fail",
":apex.apexd_test_v2",
":apex.corrupted_b146895998",
+ ":gen_key_mismatch_apex",
":gen_manifest_mismatch_apex",
":gen_manifest_mismatch_apex_no_hashtree",
":gen_corrupt_superblock_apex",
@@ -369,6 +386,7 @@
"apex_database_test.cpp",
"apex_file_test.cpp",
"apex_manifest_test.cpp",
+ "apex_preinstalled_data_test.cpp",
"apexd_verity_test.cpp",
"apexservice_test.cpp",
],
diff --git a/apexd/apex_file.cpp b/apexd/apex_file.cpp
index 0d7502b..36fd7c9 100644
--- a/apexd/apex_file.cpp
+++ b/apexd/apex_file.cpp
@@ -239,7 +239,10 @@
return Errorf("Unknown vmbeta_image_verify return value");
}
- Result<const std::string> public_key = getApexKey(apex.GetManifest().name());
+ auto& instance = ApexPreinstalledData::GetInstance();
+ // TODO(b/165948777): extract into verifier.
+ Result<const std::string> public_key =
+ instance.GetPublicKey(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 8ac1824..8555e47 100644
--- a/apexd/apex_file_test.cpp
+++ b/apexd/apex_file_test.cpp
@@ -93,7 +93,8 @@
}
TEST_P(ApexFileTest, VerifyApexVerity) {
- ASSERT_RESULT_OK(collectPreinstalledData({"/system_ext/apex"}));
+ ApexPreinstalledData& instance = ApexPreinstalledData::GetInstance();
+ ASSERT_RESULT_OK(instance.Initialize({"/system_ext/apex"}));
const std::string filePath = testDataDir + GetParam().prefix + ".apex";
Result<ApexFile> apexFile = ApexFile::Open(filePath);
ASSERT_RESULT_OK(apexFile);
diff --git a/apexd/apex_preinstalled_data.cpp b/apexd/apex_preinstalled_data.cpp
index d62c28c..dea5336 100644
--- a/apexd/apex_preinstalled_data.cpp
+++ b/apexd/apex_preinstalled_data.cpp
@@ -27,7 +27,6 @@
#include "apex_constants.h"
#include "apex_file.h"
#include "apexd_utils.h"
-#include "string_log.h"
using android::base::Error;
using android::base::Result;
@@ -35,28 +34,13 @@
namespace android {
namespace apex {
-namespace {
-
-struct ApexPreinstalledData {
- std::string name;
- std::string key;
- std::string path;
-};
-
-std::unordered_map<std::string, ApexPreinstalledData> gScannedPreinstalledData;
-
-Result<std::vector<ApexPreinstalledData>> collectPreinstalleDataFromDir(
- const std::string& dir) {
+Result<void> ApexPreinstalledData::ScanDir(const std::string& dir) {
LOG(INFO) << "Scanning " << dir << " for preinstalled data";
- std::vector<ApexPreinstalledData> ret;
if (access(dir.c_str(), F_OK) != 0 && errno == ENOENT) {
- LOG(INFO) << "... does not exist. Skipping";
- return ret;
+ LOG(INFO) << dir << " does not exist. Skipping";
+ return {};
}
- const bool scanBuiltinApexes = isPathForBuiltinApexes(dir);
- if (!scanBuiltinApexes) {
- return Error() << "Can't scan preinstalled APEX data from " << dir;
- }
+
Result<std::vector<std::string>> apex_files = FindApexFilesByName(dir);
if (!apex_files.ok()) {
return apex_files.error();
@@ -67,67 +51,63 @@
if (!apex_file.ok()) {
return Error() << "Failed to open " << file << " : " << apex_file.error();
}
- ApexPreinstalledData apexPreInstalledData;
- apexPreInstalledData.name = apex_file->GetManifest().name();
- apexPreInstalledData.key = apex_file->GetBundledPublicKey();
- apexPreInstalledData.path = apex_file->GetPath();
- ret.push_back(apexPreInstalledData);
- }
- return ret;
-}
-Result<void> updatePreinstalledData(
- const std::vector<ApexPreinstalledData>& apexes) {
- for (const ApexPreinstalledData& apex : apexes) {
- if (gScannedPreinstalledData.find(apex.name) ==
- gScannedPreinstalledData.end()) {
- gScannedPreinstalledData.insert({apex.name, apex});
+ const std::string& name = apex_file->GetManifest().name();
+ ApexData apex_data;
+ apex_data.public_key = apex_file->GetBundledPublicKey();
+ apex_data.path = apex_file->GetPath();
+
+ auto it = data_.find(name);
+ if (it == data_.end()) {
+ LOG(INFO) << "Added " << apex_data.path << " ( " << name
+ << " ) to list of pre-installed apexes";
+ data_[name] = apex_data;
+ } else if (it->second.public_key != apex_data.public_key) {
+ return Error() << "Key for package " << name
+ << " does not match with previously scanned key";
} else {
- const std::string& existing_key =
- gScannedPreinstalledData.at(apex.name).key;
- if (existing_key != apex.key) {
- return Error() << "Key for package " << apex.name
- << " does not match with previously scanned key";
- }
+ LOG(INFO) << apex_data.path << " ( " << name << " ) is already added";
}
}
return {};
}
-} // namespace
+ApexPreinstalledData& ApexPreinstalledData::GetInstance() {
+ static ApexPreinstalledData instance;
+ return instance;
+}
-Result<void> collectPreinstalledData(const std::vector<std::string>& dirs) {
+Result<void> ApexPreinstalledData::Initialize(
+ const std::vector<std::string>& dirs) {
for (const auto& dir : dirs) {
- Result<std::vector<ApexPreinstalledData>> preinstalledData =
- collectPreinstalleDataFromDir(dir);
- if (!preinstalledData.ok()) {
- return Error() << "Failed to collect keys from " << dir << " : "
- << preinstalledData.error();
- }
- Result<void> st = updatePreinstalledData(*preinstalledData);
- if (!st.ok()) {
- return st;
+ if (auto result = ScanDir(dir); !result.ok()) {
+ return result.error();
}
}
return {};
}
-Result<const std::string> getApexKey(const std::string& name) {
- if (gScannedPreinstalledData.find(name) == gScannedPreinstalledData.end()) {
+Result<const std::string> ApexPreinstalledData::GetPublicKey(
+ const std::string& name) const {
+ auto it = data_.find(name);
+ if (it == data_.end()) {
return Error() << "No preinstalled data found for package " << name;
}
- return gScannedPreinstalledData[name].key;
+ return it->second.public_key;
}
-Result<const std::string> getApexPreinstalledPath(const std::string& name) {
- if (gScannedPreinstalledData.find(name) == gScannedPreinstalledData.end()) {
+Result<const std::string> ApexPreinstalledData::GetPreinstalledPath(
+ const std::string& name) const {
+ auto it = data_.find(name);
+ if (it == data_.end()) {
return Error() << "No preinstalled data found for package " << name;
}
- return gScannedPreinstalledData[name].path;
+ return it->second.path;
}
-bool HasPreInstalledVersion(const std::string& name) {
- return gScannedPreinstalledData.find(name) != gScannedPreinstalledData.end();
+bool ApexPreinstalledData::HasPreInstalledVersion(
+ const std::string& name) const {
+ return data_.find(name) != data_.end();
}
} // namespace apex
diff --git a/apexd/apex_preinstalled_data.h b/apexd/apex_preinstalled_data.h
index 1b80aad..59aeebc 100644
--- a/apexd/apex_preinstalled_data.h
+++ b/apexd/apex_preinstalled_data.h
@@ -17,6 +17,7 @@
#pragma once
#include <string>
+#include <unordered_map>
#include <vector>
#include <android-base/result.h>
@@ -24,11 +25,61 @@
namespace android {
namespace apex {
-android::base::Result<void> collectPreinstalledData(
- const std::vector<std::string>& apex_dirs);
-android::base::Result<const std::string> getApexKey(const std::string& name);
-android::base::Result<const std::string> getApexPreinstalledPath(
- const std::string& name);
-bool HasPreInstalledVersion(const std::string& name);
+// This class encapsulates pre-installed data for all the apexes on device.
+// This data can be used to verify validity of an apex before trying to mount
+// it.
+//
+// It's expected to have a single instance of this class in a process that
+// mounts apexes (e.g. apexd, otapreopt_chroot).
+class ApexPreinstalledData final {
+ public:
+ // c-tor and d-tor are exposed for testing.
+ ApexPreinstalledData(){};
+
+ ~ApexPreinstalledData() { data_.clear(); };
+
+ // Returns a singletone instance of this class.
+ static ApexPreinstalledData& GetInstance();
+
+ // Initializes instance by collecting pre-installed data from the given
+ // |dirs|.
+ // Note: this call is **not thread safe** and is expected to be performed in a
+ // single thread during initialization of apexd. After initialization is
+ // finished, all queries to the instance are thread safe.
+ android::base::Result<void> Initialize(const std::vector<std::string>& dirs);
+
+ // Returns trusted public key for an apex with the given |name|.
+ android::base::Result<const std::string> GetPublicKey(
+ const std::string& name) const;
+
+ // Returns path to the pre-installed version of an apex with the given |name|.
+ android::base::Result<const std::string> GetPreinstalledPath(
+ const std::string& name) const;
+
+ // Checks whether there is a pre-installed version of an apex with the given
+ // |name|.
+ bool HasPreInstalledVersion(const std::string& name) const;
+
+ private:
+ // Non-copyable && non-moveable.
+ ApexPreinstalledData(const ApexPreinstalledData&) = delete;
+ ApexPreinstalledData& operator=(const ApexPreinstalledData&) = delete;
+ ApexPreinstalledData& operator=(ApexPreinstalledData&&) = delete;
+ ApexPreinstalledData(ApexPreinstalledData&&) = delete;
+
+ // Scans apexes in the given directory and adds collected data into |data_|.
+ android::base::Result<void> ScanDir(const std::string& dir);
+
+ // Internal struct to hold pre-installed data for the given apex.
+ struct ApexData {
+ // Public key of this apex.
+ std::string public_key;
+ // Path to the pre-installed version of this apex.
+ std::string path;
+ };
+
+ std::unordered_map<std::string, ApexData> data_;
+};
+
} // namespace apex
} // namespace android
diff --git a/apexd/apex_preinstalled_data_test.cpp b/apexd/apex_preinstalled_data_test.cpp
new file mode 100644
index 0000000..0e9de06
--- /dev/null
+++ b/apexd/apex_preinstalled_data_test.cpp
@@ -0,0 +1,110 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <filesystem>
+#include <string>
+
+#include <errno.h>
+#include <sys/stat.h>
+
+#include <android-base/file.h>
+#include <android-base/logging.h>
+#include <android-base/stringprintf.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "apex_file.h"
+#include "apex_preinstalled_data.h"
+#include "apexd_test_utils.h"
+#include "apexd_verity.h"
+
+namespace android {
+namespace apex {
+
+using namespace std::literals;
+
+namespace fs = std::filesystem;
+
+using android::apex::testing::IsOk;
+using android::base::GetExecutableDirectory;
+using android::base::StringPrintf;
+using ::testing::HasSubstr;
+
+static std::string GetTestDataDir() { return GetExecutableDirectory(); }
+static std::string GetTestFile(const std::string& name) {
+ return GetTestDataDir() + "/" + name;
+}
+
+TEST(ApexPreinstalledDataTest, InitializeSuccess) {
+ // Prepare test data.
+ TemporaryDir td;
+ fs::copy(GetTestFile("apex.apexd_test.apex"), td.path);
+ fs::copy(GetTestFile("apex.apexd_test_different_app.apex"), td.path);
+
+ ApexPreinstalledData instance;
+ ASSERT_TRUE(IsOk(instance.Initialize({td.path})));
+
+ // Now test that apexes were scanned correctly;
+ auto test_fn = [&](const std::string& apex_name) {
+ auto apex = ApexFile::Open(GetTestFile(apex_name));
+ ASSERT_TRUE(IsOk(apex));
+
+ {
+ auto ret = instance.GetPublicKey(apex->GetManifest().name());
+ ASSERT_TRUE(IsOk(ret));
+ ASSERT_EQ(apex->GetBundledPublicKey(), *ret);
+ }
+
+ {
+ auto ret = instance.GetPreinstalledPath(apex->GetManifest().name());
+ ASSERT_TRUE(IsOk(ret));
+ ASSERT_EQ(StringPrintf("%s/%s", td.path, apex_name.c_str()), *ret);
+ }
+
+ ASSERT_TRUE(instance.HasPreInstalledVersion(apex->GetManifest().name()));
+ };
+
+ test_fn("apex.apexd_test.apex");
+ test_fn("apex.apexd_test_different_app.apex");
+}
+
+TEST(ApexPreinstalledDataTest, InitializeFailureCorruptApex) {
+ // Prepare test data.
+ TemporaryDir td;
+ fs::copy(GetTestFile("apex.apexd_test.apex"), td.path);
+ fs::copy(GetTestFile("apex.apexd_test_corrupt_superblock_apex.apex"),
+ td.path);
+
+ ApexPreinstalledData instance;
+ ASSERT_FALSE(IsOk(instance.Initialize({td.path})));
+}
+
+TEST(ApexPreinstalledData, InitializeFailureSameNameDifferentKeys) {
+ // Prepare test data.
+ TemporaryDir td;
+ fs::copy(GetTestFile("apex.apexd_test.apex"), td.path);
+ fs::copy(GetTestFile("apex.apexd_test_different_key.apex"), td.path);
+
+ ApexPreinstalledData instance;
+ auto result = instance.Initialize({td.path});
+
+ ASSERT_FALSE(IsOk(result));
+ ASSERT_THAT(result.error().message(),
+ HasSubstr("does not match with previously scanned key"));
+}
+
+} // namespace apex
+} // namespace android
diff --git a/apexd/apexd.cpp b/apexd/apexd.cpp
index 33f9b57..0a359e1 100644
--- a/apexd/apexd.cpp
+++ b/apexd/apexd.cpp
@@ -1121,7 +1121,9 @@
std::vector<com::android::apex::ApexInfo> apexInfos;
auto convertToAutogen = [&apexInfos](const ApexFile& apex, bool isActive) {
- auto preinstalledPath = getApexPreinstalledPath(apex.GetManifest().name());
+ auto preinstalledPath =
+ ApexPreinstalledData::GetInstance().GetPreinstalledPath(
+ apex.GetManifest().name());
std::optional<std::string> preinstalledModulePath;
if (preinstalledPath.ok()) {
preinstalledModulePath = *preinstalledPath;
@@ -1311,7 +1313,8 @@
}
bool ShouldActivateApexOnData(const ApexFile& apex) {
- return HasPreInstalledVersion(apex.GetManifest().name());
+ return ApexPreinstalledData::GetInstance().HasPreInstalledVersion(
+ apex.GetManifest().name());
}
} // namespace
@@ -1937,16 +1940,17 @@
<< preAllocate.error();
}
- std::vector<std::string> bootstrap_apex_dirs{
+ ApexPreinstalledData& instance = ApexPreinstalledData::GetInstance();
+ static const std::vector<std::string> kBootstrapApexDirs{
kApexPackageSystemDir, kApexPackageSystemExtDir, kApexPackageVendorDir};
- Result<void> status = collectPreinstalledData(bootstrap_apex_dirs);
+ Result<void> status = instance.Initialize(kBootstrapApexDirs);
if (!status.ok()) {
LOG(ERROR) << "Failed to collect APEX keys : " << status.error();
return 1;
}
// Activate built-in APEXes for processes launched before /data is mounted.
- for (const auto& dir : bootstrap_apex_dirs) {
+ for (const auto& dir : kBootstrapApexDirs) {
auto scan_status = ScanApexFiles(dir.c_str());
if (!scan_status.ok()) {
LOG(ERROR) << "Failed to scan APEX files in " << dir << " : "
@@ -1997,7 +2001,8 @@
void initialize(CheckpointInterface* checkpoint_service) {
initializeVold(checkpoint_service);
- Result<void> status = collectPreinstalledData(kApexPackageBuiltinDirs);
+ ApexPreinstalledData& instance = ApexPreinstalledData::GetInstance();
+ Result<void> status = instance.Initialize(kApexPackageBuiltinDirs);
if (!status.ok()) {
LOG(ERROR) << "Failed to collect APEX keys : " << status.error();
return;
diff --git a/apexd/apexd_verity_test.cpp b/apexd/apexd_verity_test.cpp
index cf2317c..33fd782 100644
--- a/apexd/apexd_verity_test.cpp
+++ b/apexd/apexd_verity_test.cpp
@@ -45,7 +45,8 @@
}
TEST(ApexdVerityTest, ReusesHashtree) {
- ASSERT_TRUE(IsOk(collectPreinstalledData({"/system_ext/apex"})));
+ ApexPreinstalledData& instance = ApexPreinstalledData::GetInstance();
+ ASSERT_TRUE(IsOk(instance.Initialize({"/system_ext/apex"})));
TemporaryDir td;
auto apex = ApexFile::Open(GetTestFile("apex.apexd_test_no_hashtree.apex"));
@@ -78,7 +79,8 @@
}
TEST(ApexdVerityTest, RegenerateHashree) {
- ASSERT_TRUE(IsOk(collectPreinstalledData({"/system_ext/apex"})));
+ ApexPreinstalledData& instance = ApexPreinstalledData::GetInstance();
+ ASSERT_TRUE(IsOk(instance.Initialize({"/system_ext/apex"})));
TemporaryDir td;
auto apex = ApexFile::Open(GetTestFile("apex.apexd_test_no_hashtree.apex"));
diff --git a/apexd/apexservice.cpp b/apexd/apexservice.cpp
index 034a08e..75bead0 100644
--- a/apexd/apexservice.cpp
+++ b/apexd/apexservice.cpp
@@ -275,7 +275,8 @@
out.isFactory = package.IsBuiltin();
out.isActive = false;
Result<std::string> preinstalledPath =
- getApexPreinstalledPath(package.GetManifest().name());
+ ApexPreinstalledData::GetInstance().GetPreinstalledPath(
+ package.GetManifest().name());
if (preinstalledPath.ok()) {
out.preinstalledModulePath = *preinstalledPath;
}
@@ -562,7 +563,8 @@
!root.isOk()) {
return root;
}
- if (auto res = ::android::apex::collectPreinstalledData(paths); !res) {
+ ApexPreinstalledData& instance = ApexPreinstalledData::GetInstance();
+ if (auto res = instance.Initialize(paths); !res) {
return BinderStatus::fromExceptionCode(
BinderStatus::EX_SERVICE_SPECIFIC,
String8(res.error().message().c_str()));
