Skip writing verity if already written.
Computing FEC on device could take up to 3 minutes depending on
partition size, we should skip it if it's already written.
This is similar to how we skip postinstall for postponed OTA, but we
don't require passing additional header here because we can verify the
correctness of the verity data within update_engine itself.
Bug: 28171891
Test: update_engine_unittests
Change-Id: Ie9883e2260d95c05aec169dd1fde12beea0bdade
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 30fe763..f7323f9 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -1860,6 +1860,7 @@
prefs->SetInt64(kPrefsManifestSignatureSize, -1);
prefs->SetInt64(kPrefsResumedUpdateFailures, 0);
prefs->Delete(kPrefsPostInstallSucceeded);
+ prefs->Delete(kPrefsVerityWritten);
}
return true;
}
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index d74d81d..c9cb5af 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -134,7 +134,8 @@
hasher_ = std::make_unique<HashCalculator>();
offset_ = 0;
- if (verifier_step_ == VerifierStep::kVerifyTargetHash) {
+ if (verifier_step_ == VerifierStep::kVerifyTargetHash &&
+ install_plan_.write_verity) {
if (!verity_writer_->Init(partition)) {
Cleanup(ErrorCode::kVerityCalculationError);
return;
@@ -201,7 +202,8 @@
return;
}
- if (verifier_step_ == VerifierStep::kVerifyTargetHash) {
+ if (verifier_step_ == VerifierStep::kVerifyTargetHash &&
+ install_plan_.write_verity) {
if (!verity_writer_->Update(offset_, buffer_.data(), bytes_read)) {
Cleanup(ErrorCode::kVerityCalculationError);
return;
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index 15ec77a..7fa61c0 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -49,6 +49,8 @@
// Returns true iff test has completed successfully.
bool DoTest(bool terminate_early, bool hash_fail);
+ void BuildActions(const InstallPlan& install_plan);
+
brillo::FakeMessageLoop loop_{nullptr};
ActionProcessor processor_;
};
@@ -136,20 +138,10 @@
}
install_plan.partitions = {part};
- auto feeder_action = std::make_unique<ObjectFeederAction<InstallPlan>>();
- feeder_action->set_obj(install_plan);
- auto copier_action = std::make_unique<FilesystemVerifierAction>();
- auto collector_action =
- std::make_unique<ObjectCollectorAction<InstallPlan>>();
-
- BondActions(feeder_action.get(), copier_action.get());
- BondActions(copier_action.get(), collector_action.get());
+ BuildActions(install_plan);
FilesystemVerifierActionTestDelegate delegate;
processor_.set_delegate(&delegate);
- processor_.EnqueueAction(std::move(feeder_action));
- processor_.EnqueueAction(std::move(copier_action));
- processor_.EnqueueAction(std::move(collector_action));
loop_.PostTask(FROM_HERE,
base::Bind(
@@ -195,6 +187,23 @@
return success;
}
+void FilesystemVerifierActionTest::BuildActions(
+ const InstallPlan& install_plan) {
+ auto feeder_action = std::make_unique<ObjectFeederAction<InstallPlan>>();
+ auto verifier_action = std::make_unique<FilesystemVerifierAction>();
+ auto collector_action =
+ std::make_unique<ObjectCollectorAction<InstallPlan>>();
+
+ feeder_action->set_obj(install_plan);
+
+ BondActions(feeder_action.get(), verifier_action.get());
+ BondActions(verifier_action.get(), collector_action.get());
+
+ processor_.EnqueueAction(std::move(feeder_action));
+ processor_.EnqueueAction(std::move(verifier_action));
+ processor_.EnqueueAction(std::move(collector_action));
+}
+
class FilesystemVerifierActionTest2Delegate : public ActionProcessorDelegate {
public:
void ActionCompleted(ActionProcessor* processor,
@@ -210,10 +219,6 @@
};
TEST_F(FilesystemVerifierActionTest, MissingInputObjectTest) {
- FilesystemVerifierActionTest2Delegate delegate;
-
- processor_.set_delegate(&delegate);
-
auto copier_action = std::make_unique<FilesystemVerifierAction>();
auto collector_action =
std::make_unique<ObjectCollectorAction<InstallPlan>>();
@@ -222,6 +227,10 @@
processor_.EnqueueAction(std::move(copier_action));
processor_.EnqueueAction(std::move(collector_action));
+
+ FilesystemVerifierActionTest2Delegate delegate;
+ processor_.set_delegate(&delegate);
+
processor_.StartProcessing();
EXPECT_FALSE(processor_.IsRunning());
EXPECT_TRUE(delegate.ran_);
@@ -229,10 +238,6 @@
}
TEST_F(FilesystemVerifierActionTest, NonExistentDriveTest) {
- FilesystemVerifierActionTest2Delegate delegate;
-
- processor_.set_delegate(&delegate);
-
InstallPlan install_plan;
InstallPlan::Partition part;
part.name = "nope";
@@ -240,19 +245,11 @@
part.target_path = "/no/such/file";
install_plan.partitions = {part};
- auto feeder_action = std::make_unique<ObjectFeederAction<InstallPlan>>();
- auto verifier_action = std::make_unique<FilesystemVerifierAction>();
- auto collector_action =
- std::make_unique<ObjectCollectorAction<InstallPlan>>();
+ BuildActions(install_plan);
- feeder_action->set_obj(install_plan);
+ FilesystemVerifierActionTest2Delegate delegate;
+ processor_.set_delegate(&delegate);
- BondActions(feeder_action.get(), verifier_action.get());
- BondActions(verifier_action.get(), collector_action.get());
-
- processor_.EnqueueAction(std::move(feeder_action));
- processor_.EnqueueAction(std::move(verifier_action));
- processor_.EnqueueAction(std::move(collector_action));
processor_.StartProcessing();
EXPECT_FALSE(processor_.IsRunning());
EXPECT_TRUE(delegate.ran_);
@@ -278,9 +275,6 @@
#ifdef __ANDROID__
TEST_F(FilesystemVerifierActionTest, WriteVerityTest) {
- FilesystemVerifierActionTestDelegate delegate;
- processor_.set_delegate(&delegate);
-
test_utils::ScopedTempFile part_file("part_file.XXXXXX");
constexpr size_t filesystem_size = 200 * 4096;
constexpr size_t part_size = 256 * 4096;
@@ -324,19 +318,10 @@
0x70, 0xba, 0xed, 0x27, 0xe2, 0xae};
install_plan.partitions = {part};
- auto feeder_action = std::make_unique<ObjectFeederAction<InstallPlan>>();
- auto verifier_action = std::make_unique<FilesystemVerifierAction>();
- auto collector_action =
- std::make_unique<ObjectCollectorAction<InstallPlan>>();
+ BuildActions(install_plan);
- feeder_action->set_obj(install_plan);
-
- BondActions(feeder_action.get(), verifier_action.get());
- BondActions(verifier_action.get(), collector_action.get());
-
- processor_.EnqueueAction(std::move(feeder_action));
- processor_.EnqueueAction(std::move(verifier_action));
- processor_.EnqueueAction(std::move(collector_action));
+ FilesystemVerifierActionTestDelegate delegate;
+ processor_.set_delegate(&delegate);
loop_.PostTask(
FROM_HERE,
@@ -351,4 +336,49 @@
}
#endif // __ANDROID__
+TEST_F(FilesystemVerifierActionTest, SkipWriteVerityTest) {
+ test_utils::ScopedTempFile part_file("part_file.XXXXXX");
+ constexpr size_t filesystem_size = 200 * 4096;
+ constexpr size_t part_size = 256 * 4096;
+ brillo::Blob part_data(part_size);
+ test_utils::FillWithData(&part_data);
+ ASSERT_TRUE(test_utils::WriteFileVector(part_file.path(), part_data));
+ string target_path;
+ test_utils::ScopedLoopbackDeviceBinder target_device(
+ part_file.path(), true, &target_path);
+
+ InstallPlan install_plan;
+ install_plan.write_verity = false;
+ InstallPlan::Partition part;
+ part.name = "part";
+ part.target_path = target_path;
+ part.target_size = part_size;
+ part.block_size = 4096;
+ part.hash_tree_data_offset = 0;
+ part.hash_tree_data_size = filesystem_size;
+ part.hash_tree_offset = filesystem_size;
+ part.hash_tree_size = 3 * 4096;
+ part.fec_data_offset = 0;
+ part.fec_data_size = filesystem_size + part.hash_tree_size;
+ part.fec_offset = part.fec_data_size;
+ part.fec_size = 2 * 4096;
+ EXPECT_TRUE(HashCalculator::RawHashOfData(part_data, &part.target_hash));
+ install_plan.partitions = {part};
+
+ BuildActions(install_plan);
+
+ FilesystemVerifierActionTestDelegate delegate;
+ processor_.set_delegate(&delegate);
+
+ loop_.PostTask(
+ FROM_HERE,
+ base::Bind(
+ [](ActionProcessor* processor) { processor->StartProcessing(); },
+ base::Unretained(&processor_)));
+ loop_.Run();
+
+ EXPECT_FALSE(processor_.IsRunning());
+ EXPECT_TRUE(delegate.ran());
+ EXPECT_EQ(ErrorCode::kSuccess, delegate.code());
+}
} // namespace chromeos_update_engine
diff --git a/payload_consumer/install_plan.cc b/payload_consumer/install_plan.cc
index 561e294..5f2697b 100644
--- a/payload_consumer/install_plan.cc
+++ b/payload_consumer/install_plan.cc
@@ -90,7 +90,9 @@
<< ", powerwash_required: " << utils::ToString(powerwash_required)
<< ", switch_slot_on_reboot: "
<< utils::ToString(switch_slot_on_reboot)
- << ", run_post_install: " << utils::ToString(run_post_install);
+ << ", run_post_install: " << utils::ToString(run_post_install)
+ << ", is_rollback: " << utils::ToString(is_rollback)
+ << ", write_verity: " << utils::ToString(write_verity);
}
bool InstallPlan::LoadPartitionsFromSlots(BootControlInterface* boot_control) {
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index 85c97b5..f56f63c 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -146,6 +146,10 @@
// True if this update is a rollback.
bool is_rollback{false};
+ // True if the update should write verity.
+ // False otherwise.
+ bool write_verity{true};
+
// If not blank, a base-64 encoded representation of the PEM-encoded
// public key in the response.
std::string public_key_rsa;