From a89c21c9e5640ac5781924cd33dc479c10f62076 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 11 Sep 2018 14:50:02 -0700 Subject: [PATCH 1/4] Start charger at /system/bin/charger. Bug: 114042635 Test: Boot into recovery mode on walleye. Check that charger keeps working. Change-Id: I818536a6d261c860dd6f6c08774c2355e14f4236 --- etc/init.rc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etc/init.rc b/etc/init.rc index fa8fe269..0822aba0 100644 --- a/etc/init.rc +++ b/etc/init.rc @@ -73,7 +73,7 @@ service ueventd /system/bin/ueventd critical seclabel u:r:ueventd:s0 -service charger /charger -r +service charger /system/bin/charger critical seclabel u:r:charger:s0 From aaa6103ae72985d061432745e668df9ca29d6ac2 Mon Sep 17 00:00:00 2001 From: xunchang Date: Wed, 13 Mar 2019 14:21:48 -0700 Subject: [PATCH 2/4] 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 --- 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 0a594037..e27e58c2 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,52 +107,14 @@ 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()); + // CareMap in text format is no longer supported. + ASSERT_FALSE(verifier_.ParseCareMap()); } TEST_F(UpdateVerifierTest, verify_image_empty_care_map) { 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_)); - ASSERT_FALSE(verifier_.ParseCareMap()); -} - TEST_F(UpdateVerifierTest, verify_image_protobuf_care_map_smoke) { // This test relies on dm-verity support. if (!verity_supported) { diff --git a/update_verifier/include/update_verifier/update_verifier.h b/update_verifier/include/update_verifier/update_verifier.h index b00890e8..4c64b1ea 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 5e5eac7a..28c3fe78 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."; From c89c394b46832ea32625d11358efa3e587461765 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 13 Mar 2019 15:30:13 -0700 Subject: [PATCH 3/4] update_verifier: Add some missing #include's. for uint8_t; for free(3); for std::thread. Test: mmma -j bootable/recovery Test: Run unit tests on crosshatch. Change-Id: Id99b29b3d514f4e453983599c8b1aa6b0fab4ef8 --- update_verifier/update_verifier.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/update_verifier/update_verifier.cpp b/update_verifier/update_verifier.cpp index 5e5eac7a..ce93a467 100644 --- a/update_verifier/update_verifier.cpp +++ b/update_verifier/update_verifier.cpp @@ -38,17 +38,19 @@ */ #include "update_verifier/update_verifier.h" -#include #include #include #include +#include #include +#include #include #include #include #include +#include #include #include @@ -57,6 +59,7 @@ #include #include #include +#include #include #include #include From 37304f3cc920804c97d52df929b9871c057c55e0 Mon Sep 17 00:00:00 2001 From: xunchang Date: Tue, 12 Mar 2019 12:40:14 -0700 Subject: [PATCH 4/4] Implement FilePackage class This is another implementation of the Package class. And we will later need it when reading the package from FUSE. Bug: 127071893 Test: unit tests pass, sideload a file package on sailfish Change-Id: I3de5d5ef60b29c8b73517d6de3498459d7d95975 --- Android.bp | 25 ++++++- package.cpp | 114 ++++++++++++++++++++++++++++- package.h | 13 ++-- tests/Android.bp | 2 + tests/component/verifier_test.cpp | 15 ++-- tests/unit/package_test.cpp | 117 ++++++++++++++++++++++++++++++ verifier.cpp | 2 - verifier.h | 2 + 8 files changed, 273 insertions(+), 17 deletions(-) create mode 100644 tests/unit/package_test.cpp diff --git a/Android.bp b/Android.bp index 10f6c797..4c7ce52e 100644 --- a/Android.bp +++ b/Android.bp @@ -151,6 +151,7 @@ cc_defaults { static_libs: [ "librecovery_fastboot", "libminui", + "libpackage", "libverifier", "libotautil", @@ -175,7 +176,6 @@ cc_library_static { "fsck_unshare_blocks.cpp", "fuse_sdcard_provider.cpp", "install.cpp", - "package.cpp", "recovery.cpp", "roots.cpp", ], @@ -210,6 +210,29 @@ cc_library_static { ], } +cc_library_static { + name: "libpackage", + recovery_available: true, + + defaults: [ + "recovery_defaults", + ], + + srcs: [ + "package.cpp", + ], + + shared_libs: [ + "libbase", + "libcrypto", + "libziparchive", + ], + + static_libs: [ + "libotautil", + ], +} + cc_binary { name: "recovery", recovery: true, diff --git a/package.cpp b/package.cpp index d4042785..6c7289f3 100644 --- a/package.cpp +++ b/package.cpp @@ -17,10 +17,12 @@ #include "package.h" #include +#include +#include #include #include -#include +#include #include "otautil/error_code.h" #include "otautil/sysutil.h" @@ -67,14 +69,38 @@ class MemoryPackage : public Package { ZipArchiveHandle zip_handle_; }; -// TODO(xunchang) Implement the PackageFromFd. - void Package::SetProgress(float progress) { if (set_progress_) { set_progress_(progress); } } +class FilePackage : public Package { + public: + FilePackage(android::base::unique_fd&& fd, uint64_t file_size, const std::string& path, + const std::function& set_progress); + + ~FilePackage() override; + + uint64_t GetPackageSize() const override { + return package_size_; + } + + bool ReadFullyAtOffset(uint8_t* buffer, uint64_t byte_count, uint64_t offset) override; + + ZipArchiveHandle GetZipArchiveHandle() override; + + bool UpdateHashAtOffset(const std::vector& hashers, uint64_t start, + uint64_t length) override; + + private: + android::base::unique_fd fd_; // The underlying fd to the open package. + uint64_t package_size_; + std::string path_; // The physical path to the package. + + ZipArchiveHandle zip_handle_; +}; + std::unique_ptr Package::CreateMemoryPackage( const std::string& path, const std::function& set_progress) { std::unique_ptr mmap = std::make_unique(); @@ -86,6 +112,23 @@ std::unique_ptr Package::CreateMemoryPackage( return std::make_unique(path, std::move(mmap), set_progress); } +std::unique_ptr Package::CreateFilePackage( + const std::string& path, const std::function& set_progress) { + android::base::unique_fd fd(open(path.c_str(), O_RDONLY)); + if (fd == -1) { + PLOG(ERROR) << "Failed to open " << path; + return nullptr; + } + + off64_t file_size = lseek64(fd.get(), 0, SEEK_END); + if (file_size == -1) { + PLOG(ERROR) << "Failed to get the package size"; + return nullptr; + } + + return std::make_unique(std::move(fd), file_size, path, set_progress); +} + std::unique_ptr Package::CreateMemoryPackage( std::vector content, const std::function& set_progress) { return std::make_unique(std::move(content), set_progress); @@ -152,3 +195,68 @@ ZipArchiveHandle MemoryPackage::GetZipArchiveHandle() { return zip_handle_; } + +FilePackage::FilePackage(android::base::unique_fd&& fd, uint64_t file_size, const std::string& path, + const std::function& set_progress) + : fd_(std::move(fd)), package_size_(file_size), path_(path), zip_handle_(nullptr) { + set_progress_ = set_progress; +} + +FilePackage::~FilePackage() { + if (zip_handle_) { + CloseArchive(zip_handle_); + } +} + +bool FilePackage::ReadFullyAtOffset(uint8_t* buffer, uint64_t byte_count, uint64_t offset) { + if (byte_count > package_size_ || offset > package_size_ - byte_count) { + LOG(ERROR) << "Out of bound read, offset: " << offset << ", size: " << byte_count + << ", total package_size: " << package_size_; + return false; + } + + if (!android::base::ReadFullyAtOffset(fd_.get(), buffer, byte_count, offset)) { + PLOG(ERROR) << "Failed to read " << byte_count << " bytes data at offset " << offset; + return false; + } + + return true; +} + +bool FilePackage::UpdateHashAtOffset(const std::vector& hashers, + uint64_t start, uint64_t length) { + if (length > package_size_ || start > package_size_ - length) { + LOG(ERROR) << "Out of bound read, offset: " << start << ", size: " << length + << ", total package_size: " << package_size_; + return false; + } + + uint64_t so_far = 0; + while (so_far < length) { + uint64_t read_size = std::min(length - so_far, 16 * MiB); + std::vector buffer(read_size); + if (!ReadFullyAtOffset(buffer.data(), read_size, start + so_far)) { + return false; + } + + for (const auto& hasher : hashers) { + hasher(buffer.data(), read_size); + } + so_far += read_size; + } + + return true; +} + +ZipArchiveHandle FilePackage::GetZipArchiveHandle() { + if (zip_handle_) { + return zip_handle_; + } + + if (auto err = OpenArchiveFd(fd_.get(), path_.c_str(), &zip_handle_); err != 0) { + LOG(ERROR) << "Can't open package" << path_ << " : " << ErrorCodeString(err); + return nullptr; + } + + return zip_handle_; +} diff --git a/package.h b/package.h index 5eef9c96..cd44d10b 100644 --- a/package.h +++ b/package.h @@ -32,6 +32,13 @@ // interface for both packages loaded in memory and packages read from fd. class Package : public VerifierInterface { public: + static std::unique_ptr CreateMemoryPackage( + const std::string& path, const std::function& set_progress); + static std::unique_ptr CreateMemoryPackage( + std::vector content, const std::function& set_progress); + static std::unique_ptr CreateFilePackage(const std::string& path, + const std::function& set_progress); + virtual ~Package() = default; // Opens the package as a zip file and returns the ZipArchiveHandle. @@ -40,12 +47,6 @@ class Package : public VerifierInterface { // Updates the progress in fraction during package verification. void SetProgress(float progress) override; - static std::unique_ptr CreateMemoryPackage( - const std::string& path, const std::function& set_progress); - - static std::unique_ptr CreateMemoryPackage( - std::vector content, const std::function& set_progress); - protected: // An optional function to update the progress. std::function set_progress_; diff --git a/tests/Android.bp b/tests/Android.bp index 898ed7d6..ef5919eb 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -77,6 +77,7 @@ librecovery_static_libs = [ "librecovery", "librecovery_fastboot", "libminui", + "libpackage", "libverifier", "libotautil", @@ -117,6 +118,7 @@ cc_test { static_libs: libapplypatch_static_libs + [ "librecovery_ui", "libminui", + "libpackage", "libverifier", "libotautil", "libupdater", diff --git a/tests/component/verifier_test.cpp b/tests/component/verifier_test.cpp index c26d76d7..c904cd03 100644 --- a/tests/component/verifier_test.cpp +++ b/tests/component/verifier_test.cpp @@ -240,8 +240,10 @@ class VerifierTest : public testing::TestWithParam> { void SetUp() override { std::vector args = GetParam(); std::string path = from_testdata_base(args[0]); - package_ = Package::CreateMemoryPackage(path, nullptr); - ASSERT_NE(nullptr, package_); + memory_package_ = Package::CreateMemoryPackage(path, nullptr); + ASSERT_NE(nullptr, memory_package_); + file_package_ = Package::CreateFilePackage(path, nullptr); + ASSERT_NE(nullptr, file_package_); for (auto it = ++args.cbegin(); it != args.cend(); ++it) { std::string public_key_file = from_testdata_base("testkey_" + *it + ".x509.pem"); @@ -250,7 +252,8 @@ class VerifierTest : public testing::TestWithParam> { } } - std::unique_ptr package_; + std::unique_ptr memory_package_; + std::unique_ptr file_package_; std::vector certs_; }; @@ -304,11 +307,13 @@ TEST(VerifierTest, BadPackage_SignatureStartOutOfBounds) { } TEST_P(VerifierSuccessTest, VerifySucceed) { - ASSERT_EQ(VERIFY_SUCCESS, verify_file(package_.get(), certs_)); + ASSERT_EQ(VERIFY_SUCCESS, verify_file(memory_package_.get(), certs_)); + ASSERT_EQ(VERIFY_SUCCESS, verify_file(file_package_.get(), certs_)); } TEST_P(VerifierFailureTest, VerifyFailure) { - ASSERT_EQ(VERIFY_FAILURE, verify_file(package_.get(), certs_)); + ASSERT_EQ(VERIFY_FAILURE, verify_file(memory_package_.get(), certs_)); + ASSERT_EQ(VERIFY_FAILURE, verify_file(file_package_.get(), certs_)); } INSTANTIATE_TEST_CASE_P(SingleKeySuccess, VerifierSuccessTest, diff --git a/tests/unit/package_test.cpp b/tests/unit/package_test.cpp new file mode 100644 index 00000000..fa492d38 --- /dev/null +++ b/tests/unit/package_test.cpp @@ -0,0 +1,117 @@ +/* + * Copyright (C) 2019 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 agree 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 + +#include +#include +#include + +#include +#include +#include +#include + +#include "common/test_constants.h" +#include "package.h" + +class PackageTest : public ::testing::Test { + protected: + void SetUp() override; + + // A list of package classes for test, including MemoryPackage and FilePackage. + std::vector> packages_; + + TemporaryFile temp_file_; // test package file. + std::string file_content_; // actual bytes of the package file. +}; + +void PackageTest::SetUp() { + std::vector entries = { "file1.txt", "file2.txt", "dir1/file3.txt" }; + FILE* file_ptr = fdopen(temp_file_.release(), "wb"); + ZipWriter writer(file_ptr); + for (const auto& entry : entries) { + ASSERT_EQ(0, writer.StartEntry(entry.c_str(), ZipWriter::kCompress)); + ASSERT_EQ(0, writer.WriteBytes(entry.c_str(), entry.size())); + ASSERT_EQ(0, writer.FinishEntry()); + } + writer.Finish(); + ASSERT_EQ(0, fclose(file_ptr)); + + ASSERT_TRUE(android::base::ReadFileToString(temp_file_.path, &file_content_)); + auto memory_package = Package::CreateMemoryPackage(temp_file_.path, nullptr); + ASSERT_TRUE(memory_package); + packages_.emplace_back(std::move(memory_package)); + + auto file_package = Package::CreateFilePackage(temp_file_.path, nullptr); + ASSERT_TRUE(file_package); + packages_.emplace_back(std::move(file_package)); +} + +TEST_F(PackageTest, ReadFullyAtOffset_success) { + for (const auto& package : packages_) { + std::vector buffer(file_content_.size()); + ASSERT_TRUE(package->ReadFullyAtOffset(buffer.data(), file_content_.size(), 0)); + ASSERT_EQ(file_content_, std::string(buffer.begin(), buffer.end())); + + ASSERT_TRUE(package->ReadFullyAtOffset(buffer.data(), file_content_.size() - 10, 10)); + ASSERT_EQ(file_content_.substr(10), std::string(buffer.begin(), buffer.end() - 10)); + } +} + +TEST_F(PackageTest, ReadFullyAtOffset_failure) { + for (const auto& package : packages_) { + std::vector buffer(file_content_.size()); + // Out of bound read. + ASSERT_FALSE(package->ReadFullyAtOffset(buffer.data(), file_content_.size(), 10)); + } +} + +TEST_F(PackageTest, UpdateHashAtOffset_sha1_hash) { + // Check that the hash matches for first half of the file. + uint64_t hash_size = file_content_.size() / 2; + std::vector expected_sha(SHA_DIGEST_LENGTH); + SHA1(reinterpret_cast(file_content_.data()), hash_size, expected_sha.data()); + + for (const auto& package : packages_) { + SHA_CTX ctx; + SHA1_Init(&ctx); + std::vector hashers{ std::bind(&SHA1_Update, &ctx, std::placeholders::_1, + std::placeholders::_2) }; + package->UpdateHashAtOffset(hashers, 0, hash_size); + + std::vector calculated_sha(SHA_DIGEST_LENGTH); + SHA1_Final(calculated_sha.data(), &ctx); + ASSERT_EQ(expected_sha, calculated_sha); + } +} + +TEST_F(PackageTest, GetZipArchiveHandle_extract_entry) { + for (const auto& package : packages_) { + ZipArchiveHandle zip = package->GetZipArchiveHandle(); + ASSERT_TRUE(zip); + + // Check that we can extract one zip entry. + std::string entry_name = "dir1/file3.txt"; + ZipString path(entry_name.c_str()); + ZipEntry entry; + ASSERT_EQ(0, FindEntry(zip, path, &entry)); + + std::vector extracted(entry_name.size()); + ASSERT_EQ(0, ExtractToMemory(zip, &entry, extracted.data(), extracted.size())); + ASSERT_EQ(entry_name, std::string(extracted.begin(), extracted.end())); + } +} diff --git a/verifier.cpp b/verifier.cpp index b6c3895c..68a011e0 100644 --- a/verifier.cpp +++ b/verifier.cpp @@ -39,8 +39,6 @@ #include "asn1_decoder.h" #include "otautil/print_sha1.h" -static constexpr size_t MiB = 1024 * 1024; - /* * Simple version of PKCS#7 SignedData extraction. This extracts the * signature OCTET STRING to be used for signature verification. diff --git a/verifier.h b/verifier.h index b80096d4..106b86b8 100644 --- a/verifier.h +++ b/verifier.h @@ -27,6 +27,8 @@ #include #include +constexpr size_t MiB = 1024 * 1024; + using HasherUpdateCallback = std::function; struct RSADeleter {