Do not include signature dummy operation in major version 2.
It only exists for compatibility reason, for major version 2, there's no
point to add this any more.
Bug: None
TEST=Applied the new payload to a device.
Change-Id: I5803ab755415a1ba3d7460d82956bfe6e9fd4547
diff --git a/delta_performer.cc b/delta_performer.cc
index eac4259..c4dc2e4 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -1349,6 +1349,20 @@
}
}
+ if (major_payload_version_ != kChromeOSMajorPayloadVersion) {
+ if (manifest_.has_old_rootfs_info() ||
+ manifest_.has_new_rootfs_info() ||
+ manifest_.has_old_kernel_info() ||
+ manifest_.has_new_kernel_info() ||
+ manifest_.install_operations_size() != 0 ||
+ manifest_.kernel_install_operations_size() != 0) {
+ LOG(ERROR) << "Manifest contains deprecated field only supported in "
+ << "major payload version 1, but the payload major version is "
+ << major_payload_version_;
+ return ErrorCode::kPayloadMismatchedType;
+ }
+ }
+
// TODO(garnold) we should be adding more and more manifest checks, such as
// partition boundaries etc (see chromium-os:37661).
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index 8c85768..7a8cdcf 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -91,6 +91,7 @@
// Test helper placed where it can easily be friended from DeltaPerformer.
void RunManifestValidation(const DeltaArchiveManifest& manifest,
+ uint64_t major_version,
bool full_payload,
ErrorCode expected) {
// The install plan is for Full or Delta.
@@ -98,6 +99,7 @@
// The Manifest we are validating.
performer_.manifest_.CopyFrom(manifest);
+ performer_.major_payload_version_ = major_version;
EXPECT_EQ(expected, performer_.ValidateManifest());
}
@@ -455,7 +457,8 @@
manifest.mutable_new_rootfs_info();
manifest.set_minor_version(kFullPayloadMinorVersion);
- RunManifestValidation(manifest, true, ErrorCode::kSuccess);
+ RunManifestValidation(manifest, kChromeOSMajorPayloadVersion, true,
+ ErrorCode::kSuccess);
}
TEST_F(DeltaPerformerTest, ValidateManifestDeltaGoodTest) {
@@ -467,22 +470,24 @@
manifest.mutable_new_rootfs_info();
manifest.set_minor_version(DeltaPerformer::kSupportedMinorPayloadVersion);
- RunManifestValidation(manifest, false, ErrorCode::kSuccess);
+ RunManifestValidation(manifest, kChromeOSMajorPayloadVersion, false,
+ ErrorCode::kSuccess);
}
TEST_F(DeltaPerformerTest, ValidateManifestFullUnsetMinorVersion) {
// The Manifest we are validating.
DeltaArchiveManifest manifest;
- RunManifestValidation(manifest, true, ErrorCode::kSuccess);
+ RunManifestValidation(manifest, DeltaPerformer::kSupportedMajorPayloadVersion,
+ true, ErrorCode::kSuccess);
}
TEST_F(DeltaPerformerTest, ValidateManifestDeltaUnsetMinorVersion) {
// The Manifest we are validating.
DeltaArchiveManifest manifest;
- RunManifestValidation(manifest, false,
- ErrorCode::kUnsupportedMinorPayloadVersion);
+ RunManifestValidation(manifest, DeltaPerformer::kSupportedMajorPayloadVersion,
+ false, ErrorCode::kUnsupportedMinorPayloadVersion);
}
TEST_F(DeltaPerformerTest, ValidateManifestFullOldKernelTest) {
@@ -493,7 +498,8 @@
manifest.mutable_new_rootfs_info();
manifest.set_minor_version(DeltaPerformer::kSupportedMinorPayloadVersion);
- RunManifestValidation(manifest, true, ErrorCode::kPayloadMismatchedType);
+ RunManifestValidation(manifest, kChromeOSMajorPayloadVersion, true,
+ ErrorCode::kPayloadMismatchedType);
}
TEST_F(DeltaPerformerTest, ValidateManifestFullOldRootfsTest) {
@@ -504,7 +510,8 @@
manifest.mutable_new_rootfs_info();
manifest.set_minor_version(DeltaPerformer::kSupportedMinorPayloadVersion);
- RunManifestValidation(manifest, true, ErrorCode::kPayloadMismatchedType);
+ RunManifestValidation(manifest, kChromeOSMajorPayloadVersion, true,
+ ErrorCode::kPayloadMismatchedType);
}
TEST_F(DeltaPerformerTest, ValidateManifestFullPartitionUpdateTest) {
@@ -515,7 +522,8 @@
partition->mutable_new_partition_info();
manifest.set_minor_version(DeltaPerformer::kSupportedMinorPayloadVersion);
- RunManifestValidation(manifest, true, ErrorCode::kPayloadMismatchedType);
+ RunManifestValidation(manifest, kBrilloMajorPayloadVersion, true,
+ ErrorCode::kPayloadMismatchedType);
}
TEST_F(DeltaPerformerTest, ValidateManifestBadMinorVersion) {
@@ -526,12 +534,11 @@
manifest.set_minor_version(DeltaPerformer::kSupportedMinorPayloadVersion +
10000);
- RunManifestValidation(manifest, false,
- ErrorCode::kUnsupportedMinorPayloadVersion);
+ RunManifestValidation(manifest, DeltaPerformer::kSupportedMajorPayloadVersion,
+ false, ErrorCode::kUnsupportedMinorPayloadVersion);
}
TEST_F(DeltaPerformerTest, BrilloMetadataSignatureSizeTest) {
- SetSupportedMajorVersion(kBrilloMajorPayloadVersion);
EXPECT_TRUE(performer_.Write(kDeltaMagic, sizeof(kDeltaMagic)));
uint64_t major_version = htobe64(kBrilloMajorPayloadVersion);
@@ -557,7 +564,6 @@
}
TEST_F(DeltaPerformerTest, BrilloVerifyMetadataSignatureTest) {
- SetSupportedMajorVersion(kBrilloMajorPayloadVersion);
brillo::Blob payload_data = GeneratePayload({}, {}, true,
kBrilloMajorPayloadVersion,
kSourceMinorPayloadVersion);
diff --git a/payload_generator/payload_file.cc b/payload_generator/payload_file.cc
index 17baf46..ae4896a 100644
--- a/payload_generator/payload_file.cc
+++ b/payload_generator/payload_file.cc
@@ -166,8 +166,9 @@
TEST_AND_RETURN_FALSE(
PayloadSigner::SignatureBlobLength(vector<string>(1, private_key_path),
&signature_blob_length));
- PayloadSigner::AddSignatureOp(next_blob_offset, signature_blob_length,
- &manifest_);
+ PayloadSigner::AddSignatureToManifest(
+ next_blob_offset, signature_blob_length,
+ major_version_ == kChromeOSMajorPayloadVersion, &manifest_);
}
// Serialize protobuf
diff --git a/payload_generator/payload_signer.cc b/payload_generator/payload_signer.cc
index fa8c4ba..f1242e1 100644
--- a/payload_generator/payload_signer.cc
+++ b/payload_generator/payload_signer.cc
@@ -128,10 +128,11 @@
LOG(INFO) << "Matching signature sizes already present.";
} else {
// Updates the manifest to include the signature operation.
- PayloadSigner::AddSignatureOp(payload.size() - metadata_size -
- metadata_signature_size,
- signature_blob.size(),
- &manifest);
+ PayloadSigner::AddSignatureToManifest(
+ payload.size() - metadata_size - metadata_signature_size,
+ signature_blob.size(),
+ major_version == kChromeOSMajorPayloadVersion,
+ &manifest);
// Updates the payload to include the new manifest.
string serialized_manifest;
@@ -198,24 +199,27 @@
} // namespace
-void PayloadSigner::AddSignatureOp(uint64_t signature_blob_offset,
- uint64_t signature_blob_length,
- DeltaArchiveManifest* manifest) {
+void PayloadSigner::AddSignatureToManifest(uint64_t signature_blob_offset,
+ uint64_t signature_blob_length,
+ bool add_dummy_op,
+ DeltaArchiveManifest* manifest) {
LOG(INFO) << "Making room for signature in file";
manifest->set_signatures_offset(signature_blob_offset);
LOG(INFO) << "set? " << manifest->has_signatures_offset();
- // Add a dummy op at the end to appease older clients
- InstallOperation* dummy_op = manifest->add_kernel_install_operations();
- dummy_op->set_type(InstallOperation::REPLACE);
- dummy_op->set_data_offset(signature_blob_offset);
manifest->set_signatures_offset(signature_blob_offset);
- dummy_op->set_data_length(signature_blob_length);
manifest->set_signatures_size(signature_blob_length);
- Extent* dummy_extent = dummy_op->add_dst_extents();
- // Tell the dummy op to write this data to a big sparse hole
- dummy_extent->set_start_block(kSparseHole);
- dummy_extent->set_num_blocks((signature_blob_length + kBlockSize - 1) /
- kBlockSize);
+ // Add a dummy op at the end to appease older clients
+ if (add_dummy_op) {
+ InstallOperation* dummy_op = manifest->add_kernel_install_operations();
+ dummy_op->set_type(InstallOperation::REPLACE);
+ dummy_op->set_data_offset(signature_blob_offset);
+ dummy_op->set_data_length(signature_blob_length);
+ Extent* dummy_extent = dummy_op->add_dst_extents();
+ // Tell the dummy op to write this data to a big sparse hole
+ dummy_extent->set_start_block(kSparseHole);
+ dummy_extent->set_num_blocks((signature_blob_length + kBlockSize - 1) /
+ kBlockSize);
+ }
}
bool PayloadSigner::LoadPayload(const std::string& payload_path,
diff --git a/payload_generator/payload_signer.h b/payload_generator/payload_signer.h
index 8691499..e7351dd 100644
--- a/payload_generator/payload_signer.h
+++ b/payload_generator/payload_signer.h
@@ -51,11 +51,13 @@
static bool VerifySignedPayload(const std::string& payload_path,
const std::string& public_key_path);
- // Adds a dummy operation that points to a signature blob located at the
- // specified offset/length.
- static void AddSignatureOp(uint64_t signature_blob_offset,
- uint64_t signature_blob_length,
- DeltaArchiveManifest* manifest);
+ // Adds specified signature offset/length to given |manifest|, also adds a
+ // dummy operation that points to a signature blob located at the specified
+ // offset/length if |add_dummy_op| is true.
+ static void AddSignatureToManifest(uint64_t signature_blob_offset,
+ uint64_t signature_blob_length,
+ bool add_dummy_op,
+ DeltaArchiveManifest* manifest);
// Given a raw |hash| and a private key in |private_key_path| calculates the
// raw signature in |out_signature|. Returns true on success, false otherwise.