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/common/constants.cc b/common/constants.cc
index 26adbe0..4bca105 100644
--- a/common/constants.cc
+++ b/common/constants.cc
@@ -93,6 +93,7 @@
const char kPrefsUpdateBootTimestampStart[] = "update-boot-timestamp-start";
const char kPrefsUpdateTimestampStart[] = "update-timestamp-start";
const char kPrefsUrlSwitchCount[] = "url-switch-count";
+const char kPrefsVerityWritten[] = "verity-written";
const char kPrefsWallClockScatteringWaitPeriod[] = "wall-clock-wait-period";
const char kPrefsWallClockStagingWaitPeriod[] =
"wall-clock-staging-wait-period";
diff --git a/common/constants.h b/common/constants.h
index 897d2e2..1057a65 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -91,6 +91,7 @@
extern const char kPrefsUpdateBootTimestampStart[];
extern const char kPrefsUpdateTimestampStart[];
extern const char kPrefsUrlSwitchCount[];
+extern const char kPrefsVerityWritten[];
extern const char kPrefsWallClockScatteringWaitPeriod[];
extern const char kPrefsWallClockStagingWaitPeriod[];
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;
diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index 97ff6d6..a8be578 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -220,13 +220,24 @@
// c) RUN_POST_INSTALL is set to 0.
if (install_plan_.is_resume && prefs_->Exists(kPrefsPostInstallSucceeded)) {
bool post_install_succeeded = false;
- prefs_->GetBoolean(kPrefsPostInstallSucceeded, &post_install_succeeded);
- if (post_install_succeeded) {
+ if (prefs_->GetBoolean(kPrefsPostInstallSucceeded,
+ &post_install_succeeded) &&
+ post_install_succeeded) {
install_plan_.run_post_install =
GetHeaderAsBool(headers[kPayloadPropertyRunPostInstall], true);
}
}
+ // Skip writing verity if we're resuming and verity has already been written.
+ install_plan_.write_verity = true;
+ if (install_plan_.is_resume && prefs_->Exists(kPrefsVerityWritten)) {
+ bool verity_written = false;
+ if (prefs_->GetBoolean(kPrefsVerityWritten, &verity_written) &&
+ verity_written) {
+ install_plan_.write_verity = false;
+ }
+ }
+
NetworkId network_id = kDefaultNetworkId;
if (!headers[kPayloadPropertyNetworkId].empty()) {
if (!base::StringToUint64(headers[kPayloadPropertyNetworkId],
@@ -495,6 +506,8 @@
}
if (type == DownloadAction::StaticType()) {
SetStatusAndNotify(UpdateStatus::FINALIZING);
+ } else if (type == FilesystemVerifierAction::StaticType()) {
+ prefs_->SetBoolean(kPrefsVerityWritten, true);
}
}