Add and enforce MinorVersion in Payload Manifest.
The MinorVersion is also set to 0 for Full payloads, and to 1 for
DeltaPayloads (for now). If the field is not set, it defaults to 0.
The default is important, since Full payloads will be generated without this
value set in production because older payload generators will be used for
Full payloads for some time to come.
If an unexpected MinorVersion is received, we will refuse to process it, and
send an kErrorCodeUnsupportedMinorPayloadVersion error result.
Add unittests to delta_performer_unittests to unittest the ValidateManifest
method (never individually tested before).
BUG=chromium:322564
TEST=Unittests
Change-Id: Icbd2ebeb739431905497e79edb4b99629c8d6f7f
Reviewed-on: https://chromium-review.googlesource.com/177823
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
diff --git a/delta_diff_generator.cc b/delta_diff_generator.cc
index 2df0de9..8d70749 100644
--- a/delta_diff_generator.cc
+++ b/delta_diff_generator.cc
@@ -28,6 +28,7 @@
#include "update_engine/bzip.h"
#include "update_engine/cycle_breaker.h"
+#include "update_engine/delta_performer.h"
#include "update_engine/extent_mapper.h"
#include "update_engine/extent_ranges.h"
#include "update_engine/file_writer.h"
@@ -1440,6 +1441,9 @@
LOG(INFO) << "Reading files...";
+ // Create empty protobuf Manifest object
+ DeltaArchiveManifest manifest;
+
vector<DeltaArchiveManifest_InstallOperation> kernel_ops;
vector<Vertex::Index> final_order;
@@ -1511,6 +1515,10 @@
&data_file_size,
&final_order,
scratch_vertex));
+
+ // Set the minor version for this payload.
+ LOG(INFO) << "Adding Delta Minor Version.";
+ manifest.set_minor_version(DeltaPerformer::kSupportedMinorPayloadVersion);
} else {
// Full update
off_t new_image_size =
@@ -1525,12 +1533,13 @@
kBlockSize,
&kernel_ops,
&final_order));
+
+ // Set the minor version for this payload.
+ LOG(INFO) << "Adding Full Minor Version.";
+ manifest.set_minor_version(DeltaPerformer::kFullPayloadMinorVersion);
}
}
- // Convert to protobuf Manifest object
- DeltaArchiveManifest manifest;
-
if (old_image_info)
*(manifest.mutable_old_image_info()) = *old_image_info;
diff --git a/delta_performer.cc b/delta_performer.cc
index eaa65d3..2af555c 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -42,6 +42,8 @@
const uint64_t DeltaPerformer::kDeltaVersionSize = 8;
const uint64_t DeltaPerformer::kDeltaManifestSizeSize = 8;
const uint64_t DeltaPerformer::kSupportedMajorPayloadVersion = 1;
+const uint64_t DeltaPerformer::kSupportedMinorPayloadVersion = 1;
+const uint64_t DeltaPerformer::kFullPayloadMinorVersion = 0;
const char DeltaPerformer::kUpdatePayloadPublicKeyPath[] =
"/usr/share/update_engine/update-payload-key.pub.pem";
@@ -891,8 +893,8 @@
}
ErrorCode DeltaPerformer::ValidateManifest() {
- // Ensure that a full update does not contain old partition hashes, which is
- // indicative of a delta.
+ // Perform assorted checks to sanity check the manifest, make sure it
+ // matches data from other sources, and that it is a supported version.
//
// TODO(garnold) in general, the presence of an old partition hash should be
// the sole indicator for a delta update, as we would generally like update
@@ -901,11 +903,28 @@
// flow (making filesystem copying happen conditionally only *after*
// downloading and parsing of the update manifest) we'll put it off for now.
// See chromium-os:7597 for further discussion.
- if (install_plan_->is_full_update &&
- (manifest_.has_old_kernel_info() || manifest_.has_old_rootfs_info())) {
- LOG(ERROR) << "Purported full payload contains old partition "
- "hash(es), aborting update";
- return kErrorCodePayloadMismatchedType;
+ if (install_plan_->is_full_update) {
+ if (manifest_.has_old_kernel_info() || manifest_.has_old_rootfs_info()) {
+ LOG(ERROR) << "Purported full payload contains old partition "
+ "hash(es), aborting update";
+ return kErrorCodePayloadMismatchedType;
+ }
+
+ if (manifest_.minor_version() != kFullPayloadMinorVersion) {
+ LOG(ERROR) << "Manifest contains minor version "
+ << manifest_.minor_version()
+ << ", but all full payloads should have version "
+ << kFullPayloadMinorVersion << ".";
+ return kErrorCodeUnsupportedMinorPayloadVersion;
+ }
+ } else {
+ if (manifest_.minor_version() != kSupportedMinorPayloadVersion) {
+ LOG(ERROR) << "Manifest contains minor version "
+ << manifest_.minor_version()
+ << " not the supported "
+ << kSupportedMinorPayloadVersion;
+ return kErrorCodeUnsupportedMinorPayloadVersion;
+ }
}
// TODO(garnold) we should be adding more and more manifest checks, such as
diff --git a/delta_performer.h b/delta_performer.h
index ca694ed..cd059e4 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -37,6 +37,8 @@
static const uint64_t kDeltaVersionSize;
static const uint64_t kDeltaManifestSizeSize;
static const uint64_t kSupportedMajorPayloadVersion;
+ static const uint64_t kSupportedMinorPayloadVersion;
+ static const uint64_t kFullPayloadMinorVersion;
static const char kUpdatePayloadPublicKeyPath[];
// Defines the granularity of progress logging in terms of how many "completed
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index 80aa064..f02d1b7 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -630,6 +630,8 @@
InstallPlan install_plan;
install_plan.hash_checks_mandatory = hash_checks_mandatory;
install_plan.metadata_size = state->metadata_size;
+ install_plan.is_full_update = full_kernel && full_rootfs;
+
LOG(INFO) << "Setting payload metadata size in Omaha = "
<< state->metadata_size;
ASSERT_TRUE(PayloadSigner::GetMetadataSignature(
@@ -950,7 +952,28 @@
&state, hash_checks_mandatory, op_hash_test, &performer);
}
-class DeltaPerformerTest : public ::testing::Test { };
+
+class DeltaPerformerTest : public ::testing::Test {
+
+ public:
+ // Test helper placed where it can easily be friended from DeltaPerformer.
+ static void RunManifestValidation(DeltaArchiveManifest& manifest,
+ bool full_payload,
+ ErrorCode expected) {
+ PrefsMock prefs;
+ InstallPlan install_plan;
+ MockSystemState mock_system_state;
+ DeltaPerformer performer(&prefs, &mock_system_state, &install_plan);
+
+ // The install plan is for Full or Delta.
+ install_plan.is_full_update = full_payload;
+
+ // The Manifest we are validating.
+ performer.manifest_.CopyFrom(manifest);
+
+ EXPECT_EQ(expected, performer.ValidateManifest());
+ }
+};
TEST(DeltaPerformerTest, ExtentsToByteStringTest) {
uint64_t test[] = {1, 1, 4, 2, kSparseHole, 1, 0, 1};
@@ -974,6 +997,83 @@
EXPECT_EQ(expected_output, actual_output);
}
+TEST(DeltaPerformerTest, ValidateManifestFullGoodTest) {
+ // The Manifest we are validating.
+ DeltaArchiveManifest manifest;
+ manifest.mutable_new_kernel_info();
+ manifest.mutable_new_rootfs_info();
+ manifest.set_minor_version(DeltaPerformer::kFullPayloadMinorVersion);
+
+ DeltaPerformerTest::RunManifestValidation(manifest, true, kErrorCodeSuccess);
+}
+
+TEST(DeltaPerformerTest, ValidateManifestDeltaGoodTest) {
+ // The Manifest we are validating.
+ DeltaArchiveManifest manifest;
+ manifest.mutable_old_kernel_info();
+ manifest.mutable_old_rootfs_info();
+ manifest.mutable_new_kernel_info();
+ manifest.mutable_new_rootfs_info();
+ manifest.set_minor_version(DeltaPerformer::kSupportedMinorPayloadVersion);
+
+ DeltaPerformerTest::RunManifestValidation(manifest, false, kErrorCodeSuccess);
+}
+
+TEST(DeltaPerformerTest, ValidateManifestFullUnsetMinorVersion) {
+ // The Manifest we are validating.
+ DeltaArchiveManifest manifest;
+
+ DeltaPerformerTest::RunManifestValidation(manifest, true, kErrorCodeSuccess);
+}
+
+TEST(DeltaPerformerTest, ValidateManifestDeltaUnsetMinorVersion) {
+ // The Manifest we are validating.
+ DeltaArchiveManifest manifest;
+
+ DeltaPerformerTest::RunManifestValidation(
+ manifest, false,
+ kErrorCodeUnsupportedMinorPayloadVersion);
+}
+
+TEST(DeltaPerformerTest, ValidateManifestFullOldKernelTest) {
+ // The Manifest we are validating.
+ DeltaArchiveManifest manifest;
+ manifest.mutable_old_kernel_info();
+ manifest.mutable_new_kernel_info();
+ manifest.mutable_new_rootfs_info();
+ manifest.set_minor_version(DeltaPerformer::kSupportedMinorPayloadVersion);
+
+ DeltaPerformerTest::RunManifestValidation(
+ manifest, true,
+ kErrorCodePayloadMismatchedType);
+}
+
+TEST(DeltaPerformerTest, ValidateManifestFullOldRootfsTest) {
+ // The Manifest we are validating.
+ DeltaArchiveManifest manifest;
+ manifest.mutable_old_rootfs_info();
+ manifest.mutable_new_kernel_info();
+ manifest.mutable_new_rootfs_info();
+ manifest.set_minor_version(DeltaPerformer::kSupportedMinorPayloadVersion);
+
+ DeltaPerformerTest::RunManifestValidation(
+ manifest, true,
+ kErrorCodePayloadMismatchedType);
+}
+
+TEST(DeltaPerformerTest, ValidateManifestBadMinorVersion) {
+ // The Manifest we are validating.
+ DeltaArchiveManifest manifest;
+
+ // Generate a bad version number.
+ manifest.set_minor_version(DeltaPerformer::kSupportedMinorPayloadVersion +
+ 10000);
+
+ DeltaPerformerTest::RunManifestValidation(
+ manifest, false,
+ kErrorCodeUnsupportedMinorPayloadVersion);
+}
+
TEST(DeltaPerformerTest, RunAsRootSmallImageTest) {
DoSmallImageTest(false, false, false, -1, kSignatureGenerator,
false);
diff --git a/update_metadata.proto b/update_metadata.proto
index 28ed0c6..6c2b888 100644
--- a/update_metadata.proto
+++ b/update_metadata.proto
@@ -168,4 +168,6 @@
optional ImageInfo old_image_info = 10;
optional ImageInfo new_image_info = 11;
+
+ optional uint32 minor_version = 12 [default = 0];
}