From 55e3d22223ada3c9da738b4ae2824c11aa0caa91 Mon Sep 17 00:00:00 2001 From: xunchang Date: Mon, 11 Mar 2019 11:28:41 -0700 Subject: Use the package class for wipe packages The wipe package used to open the zip file directly from the content string. Switch to use the interface from the new package class instead. Bug: 127071893 Test: unit tests pass Change-Id: I990e7f00c5148710722d17140bab2e343eea3b6b --- install.h | 2 +- recovery.cpp | 55 +++++++++++++++------------------------- tests/component/install_test.cpp | 5 +++- 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/install.h b/install.h index 7b6267bf1..da8aa5e39 100644 --- a/install.h +++ b/install.h @@ -57,7 +57,7 @@ bool verify_package(Package* package); bool ReadMetadataFromPackage(ZipArchiveHandle zip, std::map* metadata); // Reads the "recovery.wipe" entry in the zip archive returns a list of partitions to wipe. -std::vector GetWipePartitionList(const std::string& wipe_package); +std::vector GetWipePartitionList(Package* wipe_package); // Verifies the compatibility info in a Treble-compatible package. Returns true directly if the // entry doesn't exist. diff --git a/recovery.cpp b/recovery.cpp index 2c9f9de68..0074b6433 100644 --- a/recovery.cpp +++ b/recovery.cpp @@ -498,41 +498,36 @@ static bool secure_wipe_partition(const std::string& partition) { return true; } -static std::string ReadWipePackage(size_t wipe_package_size) { +static std::unique_ptr ReadWipePackage(size_t wipe_package_size) { if (wipe_package_size == 0) { LOG(ERROR) << "wipe_package_size is zero"; - return ""; + return nullptr; } std::string wipe_package; std::string err_str; if (!read_wipe_package(&wipe_package, wipe_package_size, &err_str)) { PLOG(ERROR) << "Failed to read wipe package" << err_str; - return ""; + return nullptr; } - return wipe_package; + + return Package::CreateMemoryPackage( + std::vector(wipe_package.begin(), wipe_package.end()), nullptr); } // Checks if the wipe package matches expectation. If the check passes, reads the list of // partitions to wipe from the package. Checks include // 1. verify the package. // 2. check metadata (ota-type, pre-device and serial number if having one). -static bool CheckWipePackage(const std::string& wipe_package) { - auto package = Package::CreateMemoryPackage( - std::vector(wipe_package.begin(), wipe_package.end()), nullptr); - - if (!package || !verify_package(package.get())) { +static bool CheckWipePackage(Package* wipe_package) { + if (!verify_package(wipe_package)) { LOG(ERROR) << "Failed to verify package"; return false; } - // TODO(xunchang) get zip archive from package. - ZipArchiveHandle zip; - if (auto err = - OpenArchiveFromMemory(const_cast(static_cast(&wipe_package[0])), - wipe_package.size(), "wipe_package", &zip); - err != 0) { - LOG(ERROR) << "Can't open wipe package : " << ErrorCodeString(err); + ZipArchiveHandle zip = wipe_package->GetZipArchiveHandle(); + if (!zip) { + LOG(ERROR) << "Failed to get ZipArchiveHandle"; return false; } @@ -542,19 +537,13 @@ static bool CheckWipePackage(const std::string& wipe_package) { return false; } - int result = CheckPackageMetadata(metadata, OtaType::BRICK); - CloseArchive(zip); - - return result == 0; + return CheckPackageMetadata(metadata, OtaType::BRICK) == 0; } -std::vector GetWipePartitionList(const std::string& wipe_package) { - ZipArchiveHandle zip; - if (auto err = - OpenArchiveFromMemory(const_cast(static_cast(&wipe_package[0])), - wipe_package.size(), "wipe_package", &zip); - err != 0) { - LOG(ERROR) << "Can't open wipe package : " << ErrorCodeString(err); +std::vector GetWipePartitionList(Package* wipe_package) { + ZipArchiveHandle zip = wipe_package->GetZipArchiveHandle(); + if (!zip) { + LOG(ERROR) << "Failed to get ZipArchiveHandle"; return {}; } @@ -571,7 +560,6 @@ std::vector GetWipePartitionList(const std::string& wipe_package) { err != 0) { LOG(ERROR) << "Failed to extract " << RECOVERY_WIPE_ENTRY_NAME << ": " << ErrorCodeString(err); - CloseArchive(zip); return {}; } } else { @@ -581,7 +569,6 @@ std::vector GetWipePartitionList(const std::string& wipe_package) { static constexpr const char* RECOVERY_WIPE_ON_DEVICE = "/etc/recovery.wipe"; if (!android::base::ReadFileToString(RECOVERY_WIPE_ON_DEVICE, &partition_list_content)) { PLOG(ERROR) << "failed to read \"" << RECOVERY_WIPE_ON_DEVICE << "\""; - CloseArchive(zip); return {}; } } @@ -597,7 +584,6 @@ std::vector GetWipePartitionList(const std::string& wipe_package) { result.push_back(line); } - CloseArchive(zip); return result; } @@ -606,17 +592,18 @@ static bool wipe_ab_device(size_t wipe_package_size) { ui->SetBackground(RecoveryUI::ERASING); ui->SetProgressType(RecoveryUI::INDETERMINATE); - std::string wipe_package = ReadWipePackage(wipe_package_size); - if (wipe_package.empty()) { + auto wipe_package = ReadWipePackage(wipe_package_size); + if (!wipe_package) { + LOG(ERROR) << "Failed to open wipe package"; return false; } - if (!CheckWipePackage(wipe_package)) { + if (!CheckWipePackage(wipe_package.get())) { LOG(ERROR) << "Failed to verify wipe package"; return false; } - std::vector partition_list = GetWipePartitionList(wipe_package); + auto partition_list = GetWipePartitionList(wipe_package.get()); if (partition_list.empty()) { LOG(ERROR) << "Empty wipe ab partition list"; return false; diff --git a/tests/component/install_test.cpp b/tests/component/install_test.cpp index 117813694..969805b42 100644 --- a/tests/component/install_test.cpp +++ b/tests/component/install_test.cpp @@ -120,7 +120,10 @@ TEST(InstallTest, read_wipe_ab_partition_list) { std::string wipe_package; ASSERT_TRUE(android::base::ReadFileToString(temp_file.path, &wipe_package)); - std::vector read_partition_list = GetWipePartitionList(wipe_package); + auto package = Package::CreateMemoryPackage( + std::vector(wipe_package.begin(), wipe_package.end()), nullptr); + + auto read_partition_list = GetWipePartitionList(package.get()); std::vector expected = { "/dev/block/bootdevice/by-name/system_a", "/dev/block/bootdevice/by-name/system_b", "/dev/block/bootdevice/by-name/vendor_a", "/dev/block/bootdevice/by-name/vendor_b", -- cgit v1.2.3 From aab4b829f46ad6aaeaf86a4886b95a10516dc394 Mon Sep 17 00:00:00 2001 From: xunchang Date: Wed, 13 Mar 2019 14:21:48 -0700 Subject: Update_verifier: Remove the support for legacy text format CareMap We have already switched to the protobuf format for new builds, and the downgrade packages will require a data wipe. So it should be safe to drop the support for text format. This also helps to save the issue when users sideload a package with a pending OTA, because the new CareMap contains the fingerprint of the intended build. Bug: 128536706 Test: unit tests pass, run update_verifier with legacy CareMap Change-Id: I1c4d0e54ec591f16cc0a65dac76767725ff9e7c4 (cherry picked from commit aaa6103ae72985d061432745e668df9ca29d6ac2) --- tests/component/update_verifier_test.cpp | 44 ++------------------ .../include/update_verifier/update_verifier.h | 3 -- update_verifier/update_verifier.cpp | 47 ++-------------------- 3 files changed, 6 insertions(+), 88 deletions(-) diff --git a/tests/component/update_verifier_test.cpp b/tests/component/update_verifier_test.cpp index 0a594037c..e27e58c22 100644 --- a/tests/component/update_verifier_test.cpp +++ b/tests/component/update_verifier_test.cpp @@ -98,7 +98,7 @@ TEST_F(UpdateVerifierTest, verify_image_no_care_map) { ASSERT_FALSE(verifier_.ParseCareMap()); } -TEST_F(UpdateVerifierTest, verify_image_smoke) { +TEST_F(UpdateVerifierTest, verify_image_text_format) { // This test relies on dm-verity support. if (!verity_supported) { GTEST_LOG_(INFO) << "Test skipped on devices without dm-verity support."; @@ -107,49 +107,11 @@ TEST_F(UpdateVerifierTest, verify_image_smoke) { std::string content = "system\n2,0,1"; ASSERT_TRUE(android::base::WriteStringToFile(content, care_map_txt_)); - ASSERT_TRUE(verifier_.ParseCareMap()); - ASSERT_TRUE(verifier_.VerifyPartitions()); - - // Leading and trailing newlines should be accepted. - ASSERT_TRUE(android::base::WriteStringToFile("\n" + content + "\n\n", care_map_txt_)); - ASSERT_TRUE(verifier_.ParseCareMap()); - ASSERT_TRUE(verifier_.VerifyPartitions()); -} - -TEST_F(UpdateVerifierTest, verify_image_empty_care_map) { + // CareMap in text format is no longer supported. ASSERT_FALSE(verifier_.ParseCareMap()); } -TEST_F(UpdateVerifierTest, verify_image_wrong_lines) { - // The care map file can have only 2 / 4 / 6 lines. - ASSERT_TRUE(android::base::WriteStringToFile("line1", care_map_txt_)); - ASSERT_FALSE(verifier_.ParseCareMap()); - - ASSERT_TRUE(android::base::WriteStringToFile("line1\nline2\nline3", care_map_txt_)); - ASSERT_FALSE(verifier_.ParseCareMap()); -} - -TEST_F(UpdateVerifierTest, verify_image_malformed_care_map) { - // This test relies on dm-verity support. - if (!verity_supported) { - GTEST_LOG_(INFO) << "Test skipped on devices without dm-verity support."; - return; - } - - std::string content = "system\n2,1,0"; - ASSERT_TRUE(android::base::WriteStringToFile(content, care_map_txt_)); - ASSERT_FALSE(verifier_.ParseCareMap()); -} - -TEST_F(UpdateVerifierTest, verify_image_legacy_care_map) { - // This test relies on dm-verity support. - if (!verity_supported) { - GTEST_LOG_(INFO) << "Test skipped on devices without dm-verity support."; - return; - } - - std::string content = "/dev/block/bootdevice/by-name/system\n2,1,0"; - ASSERT_TRUE(android::base::WriteStringToFile(content, care_map_txt_)); +TEST_F(UpdateVerifierTest, verify_image_empty_care_map) { ASSERT_FALSE(verifier_.ParseCareMap()); } diff --git a/update_verifier/include/update_verifier/update_verifier.h b/update_verifier/include/update_verifier/update_verifier.h index b00890e82..4c64b1ea1 100644 --- a/update_verifier/include/update_verifier/update_verifier.h +++ b/update_verifier/include/update_verifier/update_verifier.h @@ -50,9 +50,6 @@ class UpdateVerifier { private: friend class UpdateVerifierTest; - // Parses the legacy care_map.txt in plain text format. - bool ParseCareMapPlainText(const std::string& content); - // Finds all the dm-enabled partitions, and returns a map of . std::map FindDmPartitions(); diff --git a/update_verifier/update_verifier.cpp b/update_verifier/update_verifier.cpp index 5e5eac7ab..28c3fe780 100644 --- a/update_verifier/update_verifier.cpp +++ b/update_verifier/update_verifier.cpp @@ -68,6 +68,7 @@ using android::hardware::boot::V1_0::IBootControl; using android::hardware::boot::V1_0::BoolResult; using android::hardware::boot::V1_0::CommandResult; +// TODO(xunchang) remove the prefix and use a default path instead. constexpr const char* kDefaultCareMapPrefix = "/data/ota_package/care_map"; // Find directories in format of "/sys/block/dm-X". @@ -196,51 +197,13 @@ bool UpdateVerifier::VerifyPartitions() { return true; } -bool UpdateVerifier::ParseCareMapPlainText(const std::string& content) { - // care_map file has up to six lines, where every two lines make a pair. Within each pair, the - // first line has the partition name (e.g. "system"), while the second line holds the ranges of - // all the blocks to verify. - auto lines = android::base::Split(android::base::Trim(content), "\n"); - if (lines.size() != 2 && lines.size() != 4 && lines.size() != 6) { - LOG(WARNING) << "Invalid lines in care_map: found " << lines.size() - << " lines, expecting 2 or 4 or 6 lines."; - return false; - } - - for (size_t i = 0; i < lines.size(); i += 2) { - const std::string& partition_name = lines[i]; - const std::string& range_str = lines[i + 1]; - // We're seeing an N care_map.txt. Skip the verification since it's not compatible with O - // update_verifier (the last few metadata blocks can't be read via device mapper). - if (android::base::StartsWith(partition_name, "/dev/block/")) { - LOG(WARNING) << "Found legacy care_map.txt; skipped."; - return false; - } - - // For block range string, first integer 'count' equals 2 * total number of valid ranges, - // followed by 'count' number comma separated integers. Every two integers reprensent a - // block range with the first number included in range but second number not included. - // For example '4,64536,65343,74149,74150' represents: [64536,65343) and [74149,74150). - RangeSet ranges = RangeSet::Parse(range_str); - if (!ranges) { - LOG(WARNING) << "Error parsing RangeSet string " << range_str; - return false; - } - - partition_map_.emplace(partition_name, ranges); - } - - return true; -} - bool UpdateVerifier::ParseCareMap() { partition_map_.clear(); std::string care_map_name = care_map_prefix_ + ".pb"; if (access(care_map_name.c_str(), R_OK) == -1) { - LOG(WARNING) << care_map_name - << " doesn't exist, falling back to read the care_map in plain text format."; - care_map_name = care_map_prefix_ + ".txt"; + LOG(ERROR) << care_map_name << " doesn't exist"; + return false; } android::base::unique_fd care_map_fd(TEMP_FAILURE_RETRY(open(care_map_name.c_str(), O_RDONLY))); @@ -263,10 +226,6 @@ bool UpdateVerifier::ParseCareMap() { return false; } - if (android::base::EndsWith(care_map_name, ".txt")) { - return ParseCareMapPlainText(file_content); - } - recovery_update_verifier::CareMap care_map; if (!care_map.ParseFromString(file_content)) { LOG(WARNING) << "Failed to parse " << care_map_name << " in protobuf format."; -- cgit v1.2.3