Revert "update_engine: Deprecate major version 1"
This partially reverts commit 55c75417e22d5026971276997924a345d9973bbc.
It turns out that we forgot a scenario when we deprecated major version
1. We use update_engine in lab tests (specifically
autoupdate_EndToEndTests on stable channel) to update a DUT to an
old (very old) versions using actual update payloads so we can test that
they can get updated to newer versions. However, deprecating major
version 1 in the update_engine caused trouble because we no longer can
update from a newer version to a version before M72 (to prepare the
device for update test). We need to put this feature back until we find
a better solution for it.
On this CL, we only support major version 1 in the client and only for
test (non-official) images. We don't even bother adding paygen support
for it.
This CL should be reverted once we figured out what to do with
provisioning the autoupdate end to end tests.
BUG=chromium:1043428
TEST=FEATURES=test emerge-reef update_engine
TEST=cros deployed it, then cros flash using an m71 payload, it succeeded.
Change-Id: I1fecbe3ae845b2e419f0999adc53e4732b1f7696
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2013884
Reviewed-by: Tianjie Xu <xunchang@google.com>
Reviewed-by: Sen Jiang <senj@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 3263ff7..262e8bc 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -437,7 +437,7 @@
if (!IsHeaderParsed()) {
MetadataParseResult result =
- payload_metadata_.ParsePayloadHeader(payload, error);
+ payload_metadata_.ParsePayloadHeader(payload, hardware_, error);
if (result != MetadataParseResult::kSuccess)
return result;
@@ -728,7 +728,8 @@
// In major version 2, we don't add dummy operation to the payload.
// If we already extracted the signature we should skip this step.
- if (manifest_.has_signatures_offset() && manifest_.has_signatures_size() &&
+ if (major_payload_version_ == kBrilloMajorPayloadVersion &&
+ manifest_.has_signatures_offset() && manifest_.has_signatures_size() &&
signatures_message_data_.empty()) {
if (manifest_.signatures_offset() != buffer_offset_) {
LOG(ERROR) << "Payload signatures offset points to blob offset "
@@ -763,11 +764,51 @@
}
bool DeltaPerformer::ParseManifestPartitions(ErrorCode* error) {
- partitions_.clear();
- for (const PartitionUpdate& partition : manifest_.partitions()) {
- partitions_.push_back(partition);
+ if (major_payload_version_ == kBrilloMajorPayloadVersion) {
+ partitions_.clear();
+ for (const PartitionUpdate& partition : manifest_.partitions()) {
+ partitions_.push_back(partition);
+ }
+ manifest_.clear_partitions();
+ } else if (major_payload_version_ == kChromeOSMajorPayloadVersion) {
+ LOG(INFO) << "Converting update information from old format.";
+ PartitionUpdate root_part;
+ root_part.set_partition_name(kPartitionNameRoot);
+#ifdef __ANDROID__
+ LOG(WARNING) << "Legacy payload major version provided to an Android "
+ "build. Assuming no post-install. Please use major version "
+ "2 or newer.";
+ root_part.set_run_postinstall(false);
+#else
+ root_part.set_run_postinstall(true);
+#endif // __ANDROID__
+ if (manifest_.has_old_rootfs_info()) {
+ *root_part.mutable_old_partition_info() = manifest_.old_rootfs_info();
+ manifest_.clear_old_rootfs_info();
+ }
+ if (manifest_.has_new_rootfs_info()) {
+ *root_part.mutable_new_partition_info() = manifest_.new_rootfs_info();
+ manifest_.clear_new_rootfs_info();
+ }
+ *root_part.mutable_operations() = manifest_.install_operations();
+ manifest_.clear_install_operations();
+ partitions_.push_back(std::move(root_part));
+
+ PartitionUpdate kern_part;
+ kern_part.set_partition_name(kPartitionNameKernel);
+ kern_part.set_run_postinstall(false);
+ if (manifest_.has_old_kernel_info()) {
+ *kern_part.mutable_old_partition_info() = manifest_.old_kernel_info();
+ manifest_.clear_old_kernel_info();
+ }
+ if (manifest_.has_new_kernel_info()) {
+ *kern_part.mutable_new_partition_info() = manifest_.new_kernel_info();
+ manifest_.clear_new_kernel_info();
+ }
+ *kern_part.mutable_operations() = manifest_.kernel_install_operations();
+ manifest_.clear_kernel_install_operations();
+ partitions_.push_back(std::move(kern_part));
}
- manifest_.clear_partitions();
// Fill in the InstallPlan::partitions based on the partitions from the
// payload.
@@ -928,6 +969,14 @@
TEST_AND_RETURN_FALSE(buffer_offset_ == operation.data_offset());
TEST_AND_RETURN_FALSE(buffer_.size() >= operation.data_length());
+ // Extract the signature message if it's in this operation.
+ if (ExtractSignatureMessageFromOperation(operation)) {
+ // If this is dummy replace operation, we ignore it after extracting the
+ // signature.
+ DiscardBuffer(true, 0);
+ return true;
+ }
+
// Setup the ExtentWriter stack based on the operation type.
std::unique_ptr<ExtentWriter> writer = std::make_unique<DirectExtentWriter>();
@@ -1378,6 +1427,19 @@
return true;
}
+bool DeltaPerformer::ExtractSignatureMessageFromOperation(
+ const InstallOperation& operation) {
+ if (operation.type() != InstallOperation::REPLACE ||
+ !manifest_.has_signatures_offset() ||
+ manifest_.signatures_offset() != operation.data_offset()) {
+ return false;
+ }
+ TEST_AND_RETURN_FALSE(manifest_.has_signatures_size() &&
+ manifest_.signatures_size() == operation.data_length());
+ TEST_AND_RETURN_FALSE(ExtractSignatureMessage());
+ return true;
+}
+
bool DeltaPerformer::ExtractSignatureMessage() {
TEST_AND_RETURN_FALSE(signatures_message_data_.empty());
TEST_AND_RETURN_FALSE(buffer_offset_ == manifest_.signatures_offset());
@@ -1429,11 +1491,11 @@
// Perform assorted checks to sanity check the manifest, make sure it
// matches data from other sources, and that it is a supported version.
- bool has_old_fields = std::any_of(manifest_.partitions().begin(),
- manifest_.partitions().end(),
- [](const PartitionUpdate& partition) {
- return partition.has_old_partition_info();
- });
+ bool has_old_fields =
+ (manifest_.has_old_kernel_info() || manifest_.has_old_rootfs_info());
+ for (const PartitionUpdate& partition : manifest_.partitions()) {
+ has_old_fields = has_old_fields || partition.has_old_partition_info();
+ }
// The presence of an old partition hash is the sole indicator for a delta
// update.
@@ -1475,12 +1537,16 @@
}
}
- 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 fields.";
- return ErrorCode::kPayloadMismatchedType;
+ 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;
+ }
}
if (manifest_.max_timestamp() < hardware_->GetBuildTimestamp()) {
@@ -1491,6 +1557,16 @@
return ErrorCode::kPayloadTimestampError;
}
+ if (major_payload_version_ == kChromeOSMajorPayloadVersion) {
+ if (manifest_.has_dynamic_partition_metadata()) {
+ LOG(ERROR)
+ << "Should not contain dynamic_partition_metadata for major version "
+ << kChromeOSMajorPayloadVersion
+ << ". Please use major version 2 or above.";
+ return ErrorCode::kPayloadMismatchedType;
+ }
+ }
+
// TODO(crbug.com/37661) we should be adding more and more manifest checks,
// such as partition boundaries, etc.
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index 7860747..4493c2a 100644
--- a/payload_consumer/delta_performer.h
+++ b/payload_consumer/delta_performer.h
@@ -237,6 +237,11 @@
FileDescriptorPtr ChooseSourceFD(const InstallOperation& operation,
ErrorCode* error);
+ // Extracts the payload signature message from the blob on the |operation| if
+ // the offset matches the one specified by the manifest. Returns whether the
+ // signature was extracted.
+ bool ExtractSignatureMessageFromOperation(const InstallOperation& operation);
+
// Extracts the payload signature message from the current |buffer_| if the
// offset matches the one specified by the manifest. Returns whether the
// signature was extracted.
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index f1a492b..8de70e3 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -591,7 +591,8 @@
{
EXPECT_TRUE(utils::ReadFile(state->delta_path, &state->delta));
PayloadMetadata payload_metadata;
- EXPECT_TRUE(payload_metadata.ParsePayloadHeader(state->delta));
+ EXPECT_TRUE(payload_metadata.ParsePayloadHeader(state->delta,
+ &state->fake_hardware_));
state->metadata_size = payload_metadata.GetMetadataSize();
LOG(INFO) << "Metadata size: " << state->metadata_size;
state->metadata_signature_size =
diff --git a/payload_consumer/payload_constants.cc b/payload_consumer/payload_constants.cc
index 908a893..4015a0a 100644
--- a/payload_consumer/payload_constants.cc
+++ b/payload_consumer/payload_constants.cc
@@ -20,7 +20,7 @@
namespace chromeos_update_engine {
-// const uint64_t kChromeOSMajorPayloadVersion = 1; DEPRECATED
+const uint64_t kChromeOSMajorPayloadVersion = 1;
const uint64_t kBrilloMajorPayloadVersion = 2;
const uint64_t kMinSupportedMajorPayloadVersion = kBrilloMajorPayloadVersion;
diff --git a/payload_consumer/payload_constants.h b/payload_consumer/payload_constants.h
index 888fa2a..fe823f4 100644
--- a/payload_consumer/payload_constants.h
+++ b/payload_consumer/payload_constants.h
@@ -26,7 +26,7 @@
namespace chromeos_update_engine {
// The major version used by Chrome OS.
-// extern const uint64_t kChromeOSMajorPayloadVersion; DEPRECATED
+extern const uint64_t kChromeOSMajorPayloadVersion;
// The major version used by Brillo.
extern const uint64_t kBrilloMajorPayloadVersion;
diff --git a/payload_consumer/payload_metadata.cc b/payload_consumer/payload_metadata.cc
index b83001a..69ccb46 100644
--- a/payload_consumer/payload_metadata.cc
+++ b/payload_consumer/payload_metadata.cc
@@ -20,6 +20,7 @@
#include <brillo/data_encoding.h>
+#include "update_engine/common/hardware_interface.h"
#include "update_engine/common/hash_calculator.h"
#include "update_engine/common/utils.h"
#include "update_engine/payload_consumer/payload_constants.h"
@@ -36,18 +37,36 @@
const uint64_t PayloadMetadata::kDeltaManifestSizeSize = 8;
const uint64_t PayloadMetadata::kDeltaMetadataSignatureSizeSize = 4;
-uint64_t PayloadMetadata::GetMetadataSignatureSizeOffset() const {
- return kDeltaManifestSizeOffset + kDeltaManifestSizeSize;
+bool PayloadMetadata::GetMetadataSignatureSizeOffset(
+ uint64_t* out_offset) const {
+ if (GetMajorVersion() == kBrilloMajorPayloadVersion) {
+ *out_offset = kDeltaManifestSizeOffset + kDeltaManifestSizeSize;
+ return true;
+ }
+ return false;
}
-uint64_t PayloadMetadata::GetManifestOffset() const {
- // Actual manifest begins right after the metadata signature size field.
- return kDeltaManifestSizeOffset + kDeltaManifestSizeSize +
- kDeltaMetadataSignatureSizeSize;
+bool PayloadMetadata::GetManifestOffset(uint64_t* out_offset) const {
+ // Actual manifest begins right after the manifest size field or
+ // metadata signature size field if major version >= 2.
+ if (major_payload_version_ == kChromeOSMajorPayloadVersion) {
+ *out_offset = kDeltaManifestSizeOffset + kDeltaManifestSizeSize;
+ return true;
+ }
+ if (major_payload_version_ == kBrilloMajorPayloadVersion) {
+ *out_offset = kDeltaManifestSizeOffset + kDeltaManifestSizeSize +
+ kDeltaMetadataSignatureSizeSize;
+ return true;
+ }
+ LOG(ERROR) << "Unknown major payload version: " << major_payload_version_;
+ return false;
}
MetadataParseResult PayloadMetadata::ParsePayloadHeader(
- const brillo::Blob& payload, ErrorCode* error) {
+ const brillo::Blob& payload,
+ HardwareInterface* hardware,
+ ErrorCode* error) {
+ uint64_t manifest_offset;
// Ensure we have data to cover the major payload version.
if (payload.size() < kDeltaManifestSizeOffset)
return MetadataParseResult::kInsufficientData;
@@ -59,11 +78,6 @@
return MetadataParseResult::kError;
}
- uint64_t manifest_offset = GetManifestOffset();
- // Check again with the manifest offset.
- if (payload.size() < manifest_offset)
- return MetadataParseResult::kInsufficientData;
-
// Extract the payload version from the metadata.
static_assert(sizeof(major_payload_version_) == kDeltaVersionSize,
"Major payload version size mismatch");
@@ -73,14 +87,26 @@
// Switch big endian to host.
major_payload_version_ = be64toh(major_payload_version_);
- if (major_payload_version_ < kMinSupportedMajorPayloadVersion ||
- major_payload_version_ > kMaxSupportedMajorPayloadVersion) {
+ // We only want to test major version 1 for test images.
+ if (major_payload_version_ == kChromeOSMajorPayloadVersion
+ ? hardware != nullptr && hardware->IsOfficialBuild()
+ : major_payload_version_ < kMinSupportedMajorPayloadVersion ||
+ major_payload_version_ > kMaxSupportedMajorPayloadVersion) {
LOG(ERROR) << "Bad payload format -- unsupported payload version: "
<< major_payload_version_;
*error = ErrorCode::kUnsupportedMajorPayloadVersion;
return MetadataParseResult::kError;
}
+ // Get the manifest offset now that we have payload version.
+ if (!GetManifestOffset(&manifest_offset)) {
+ *error = ErrorCode::kUnsupportedMajorPayloadVersion;
+ return MetadataParseResult::kError;
+ }
+ // Check again with the manifest offset.
+ if (payload.size() < manifest_offset)
+ return MetadataParseResult::kInsufficientData;
+
// Next, parse the manifest size.
static_assert(sizeof(manifest_size_) == kDeltaManifestSizeSize,
"manifest_size size mismatch");
@@ -97,33 +123,43 @@
return MetadataParseResult::kError;
}
- // Parse the metadata signature size.
- static_assert(
- sizeof(metadata_signature_size_) == kDeltaMetadataSignatureSizeSize,
- "metadata_signature_size size mismatch");
- uint64_t metadata_signature_size_offset = GetMetadataSignatureSizeOffset();
- memcpy(&metadata_signature_size_,
- &payload[metadata_signature_size_offset],
- kDeltaMetadataSignatureSizeSize);
- metadata_signature_size_ = be32toh(metadata_signature_size_);
+ if (GetMajorVersion() == kBrilloMajorPayloadVersion) {
+ // Parse the metadata signature size.
+ static_assert(
+ sizeof(metadata_signature_size_) == kDeltaMetadataSignatureSizeSize,
+ "metadata_signature_size size mismatch");
+ uint64_t metadata_signature_size_offset;
+ if (!GetMetadataSignatureSizeOffset(&metadata_signature_size_offset)) {
+ *error = ErrorCode::kError;
+ return MetadataParseResult::kError;
+ }
+ memcpy(&metadata_signature_size_,
+ &payload[metadata_signature_size_offset],
+ kDeltaMetadataSignatureSizeSize);
+ metadata_signature_size_ = be32toh(metadata_signature_size_);
- if (metadata_size_ + metadata_signature_size_ < metadata_size_) {
- // Overflow detected.
- LOG(ERROR) << "Overflow detected on metadata and signature size.";
- *error = ErrorCode::kDownloadInvalidMetadataSize;
- return MetadataParseResult::kError;
+ if (metadata_size_ + metadata_signature_size_ < metadata_size_) {
+ // Overflow detected.
+ LOG(ERROR) << "Overflow detected on metadata and signature size.";
+ *error = ErrorCode::kDownloadInvalidMetadataSize;
+ return MetadataParseResult::kError;
+ }
}
return MetadataParseResult::kSuccess;
}
-bool PayloadMetadata::ParsePayloadHeader(const brillo::Blob& payload) {
+bool PayloadMetadata::ParsePayloadHeader(const brillo::Blob& payload,
+ HardwareInterface* hardware) {
ErrorCode error;
- return ParsePayloadHeader(payload, &error) == MetadataParseResult::kSuccess;
+ return ParsePayloadHeader(payload, hardware, &error) ==
+ MetadataParseResult::kSuccess;
}
bool PayloadMetadata::GetManifest(const brillo::Blob& payload,
DeltaArchiveManifest* out_manifest) const {
- uint64_t manifest_offset = GetManifestOffset();
+ uint64_t manifest_offset;
+ if (!GetManifestOffset(&manifest_offset))
+ return false;
CHECK_GE(payload.size(), manifest_offset + manifest_size_);
return out_manifest->ParseFromArray(&payload[manifest_offset],
manifest_size_);
@@ -145,7 +181,7 @@
<< metadata_signature;
return ErrorCode::kDownloadMetadataSignatureError;
}
- } else {
+ } else if (major_payload_version_ == kBrilloMajorPayloadVersion) {
metadata_signature_protobuf_blob.assign(
payload.begin() + metadata_size_,
payload.begin() + metadata_size_ + metadata_signature_size_);
@@ -206,7 +242,7 @@
brillo::Blob payload;
TEST_AND_RETURN_FALSE(
utils::ReadFileChunk(payload_path, 0, kMaxPayloadHeaderSize, &payload));
- TEST_AND_RETURN_FALSE(ParsePayloadHeader(payload));
+ TEST_AND_RETURN_FALSE(ParsePayloadHeader(payload, nullptr));
if (manifest != nullptr) {
TEST_AND_RETURN_FALSE(
@@ -217,7 +253,8 @@
TEST_AND_RETURN_FALSE(GetManifest(payload, manifest));
}
- if (metadata_signatures != nullptr) {
+ if (metadata_signatures != nullptr &&
+ GetMajorVersion() >= kBrilloMajorPayloadVersion) {
payload.clear();
TEST_AND_RETURN_FALSE(utils::ReadFileChunk(
payload_path, GetMetadataSize(), GetMetadataSignatureSize(), &payload));
diff --git a/payload_consumer/payload_metadata.h b/payload_consumer/payload_metadata.h
index be43c41..3292351 100644
--- a/payload_consumer/payload_metadata.h
+++ b/payload_consumer/payload_metadata.h
@@ -26,6 +26,7 @@
#include <brillo/secure_blob.h>
#include "update_engine/common/error_code.h"
+#include "update_engine/common/hardware_interface.h"
#include "update_engine/common/platform_constants.h"
#include "update_engine/update_metadata.pb.h"
@@ -54,9 +55,11 @@
// metadata. Returns kMetadataParseError if the metadata can't be parsed given
// the payload.
MetadataParseResult ParsePayloadHeader(const brillo::Blob& payload,
+ HardwareInterface* hardware,
ErrorCode* error);
// Simpler version of the above, returns true on success.
- bool ParsePayloadHeader(const brillo::Blob& payload);
+ bool ParsePayloadHeader(const brillo::Blob& payload,
+ HardwareInterface* hardware);
// Given the |payload|, verifies that the signed hash of its metadata matches
// |metadata_signature| (if present) or the metadata signature in payload
@@ -94,12 +97,14 @@
Signatures* metadata_signatures);
private:
- // Returns the byte offset at which the manifest protobuf begins in a payload.
- uint64_t GetManifestOffset() const;
+ // Set |*out_offset| to the byte offset at which the manifest protobuf begins
+ // in a payload. Return true on success, false if the offset is unknown.
+ bool GetManifestOffset(uint64_t* out_offset) const;
- // Returns the byte offset where the size of the metadata signature is stored
- // in a payload.
- uint64_t GetMetadataSignatureSizeOffset() const;
+ // Set |*out_offset| to the byte offset where the size of the metadata
+ // signature is stored in a payload. Return true on success, if this field is
+ // not present in the payload, return false.
+ bool GetMetadataSignatureSizeOffset(uint64_t* out_offset) const;
uint64_t metadata_size_{0};
uint64_t manifest_size_{0};
diff --git a/payload_generator/payload_signer.cc b/payload_generator/payload_signer.cc
index 420329f..613202d 100644
--- a/payload_generator/payload_signer.cc
+++ b/payload_generator/payload_signer.cc
@@ -94,7 +94,7 @@
brillo::Blob payload;
TEST_AND_RETURN_FALSE(utils::ReadFile(payload_path, &payload));
PayloadMetadata payload_metadata;
- TEST_AND_RETURN_FALSE(payload_metadata.ParsePayloadHeader(payload));
+ TEST_AND_RETURN_FALSE(payload_metadata.ParsePayloadHeader(payload, nullptr));
uint64_t metadata_size = payload_metadata.GetMetadataSize();
uint32_t metadata_signature_size =
payload_metadata.GetMetadataSignatureSize();
@@ -218,7 +218,7 @@
brillo::Blob payload;
TEST_AND_RETURN_FALSE(utils::ReadFile(payload_path, &payload));
PayloadMetadata payload_metadata;
- TEST_AND_RETURN_FALSE(payload_metadata.ParsePayloadHeader(payload));
+ TEST_AND_RETURN_FALSE(payload_metadata.ParsePayloadHeader(payload, nullptr));
DeltaArchiveManifest manifest;
TEST_AND_RETURN_FALSE(payload_metadata.GetManifest(payload, &manifest));
TEST_AND_RETURN_FALSE(manifest.has_signatures_offset() &&
diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index c738e4e..e2b5a88 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -368,7 +368,7 @@
}
ErrorCode errorcode;
PayloadMetadata payload_metadata;
- if (payload_metadata.ParsePayloadHeader(metadata, &errorcode) !=
+ if (payload_metadata.ParsePayloadHeader(metadata, nullptr, &errorcode) !=
MetadataParseResult::kSuccess) {
return LogAndSetError(error,
FROM_HERE,