Enforce handling of expected payload versions only.
We have always shipped version 1 payloads, but have never checked the
version number. This change enforces that, so we refuse to process
any other version number.
The new error code kErrorCodeUnsupportedPayloadVersion is added to
report bad payload versions if they are received.
BUG=chromium:312526
TEST=Unittests.
Change-Id: I25c9f7e73c37274527bc7cc9ba9e3d9f4734326c
Reviewed-on: https://chromium-review.googlesource.com/174940
Commit-Queue: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
diff --git a/delta_performer.cc b/delta_performer.cc
index c20614d..69ffbc1 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -39,6 +39,8 @@
const uint64_t DeltaPerformer::kDeltaVersionSize = 8;
const uint64_t DeltaPerformer::kDeltaManifestSizeSize = 8;
+const uint64_t DeltaPerformer::kSupportedMajorPayloadVersion = 1;
+
const char DeltaPerformer::kUpdatePayloadPublicKeyPath[] =
"/usr/share/update_engine/update-payload-key.pub.pem";
const unsigned DeltaPerformer::kProgressLogMaxChunks = 10;
@@ -247,6 +249,11 @@
} // namespace {}
+uint64_t DeltaPerformer::GetVersionOffset() {
+ // Manifest size is stored right after the magic string and the version.
+ return strlen(kDeltaMagic);
+}
+
uint64_t DeltaPerformer::GetManifestSizeOffset() {
// Manifest size is stored right after the magic string and the version.
return strlen(kDeltaMagic) + kDeltaVersionSize;
@@ -279,8 +286,22 @@
return kMetadataParseError;
}
- // TODO(jaysri): Compare the version number and skip unknown manifest
- // versions. We don't check the version at all today.
+ // Extract the payload version from the metadata.
+ uint64_t major_payload_version;
+ COMPILE_ASSERT(sizeof(major_payload_version) == kDeltaVersionSize,
+ major_payload_version_size_mismatch);
+ memcpy(&major_payload_version,
+ &payload[GetVersionOffset()],
+ kDeltaVersionSize);
+ // switch big endian to host
+ major_payload_version = be64toh(major_payload_version);
+
+ if (major_payload_version != kSupportedMajorPayloadVersion) {
+ LOG(ERROR) << "Bad payload format -- unsupported payload version: "
+ << major_payload_version;
+ *error = kErrorCodeUnsupportedMajorPayloadVersion;
+ return kMetadataParseError;
+ }
// Next, parse the manifest size.
uint64_t manifest_size;
diff --git a/delta_performer.h b/delta_performer.h
index 7943d7a..a9cbe44 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -36,6 +36,7 @@
static const uint64_t kDeltaVersionSize;
static const uint64_t kDeltaManifestSizeSize;
+ static const uint64_t kSupportedMajorPayloadVersion;
static const char kUpdatePayloadPublicKeyPath[];
// Defines the granularity of progress logging in terms of how many "completed
@@ -163,15 +164,18 @@
public_key_path_ = public_key_path;
}
- // Returns the byte offset at which the manifest protobuf begins in a
- // payload.
- static uint64_t GetManifestOffset();
+ // Returns the byte offset at which the payload version can be found.
+ static uint64_t GetVersionOffset();
// Returns the byte offset where the size of the manifest is stored in
// a payload. This offset precedes the actual start of the manifest
// that's returned by the GetManifestOffset method.
static uint64_t GetManifestSizeOffset();
+ // Returns the byte offset at which the manifest protobuf begins in a
+ // payload.
+ static uint64_t GetManifestOffset();
+
private:
friend class DeltaPerformerTest;
FRIEND_TEST(DeltaPerformerTest, IsIdempotentOperationTest);
diff --git a/error_code.h b/error_code.h
index 65a5533..5853c1b 100644
--- a/error_code.h
+++ b/error_code.h
@@ -53,11 +53,15 @@
kErrorCodePostinstallPowerwashError = 41,
kErrorCodeUpdateCanceledByChannelChange = 42,
kErrorCodePostinstallFirmwareRONotUpdatable = 43,
+ kErrorCodeUnsupportedMajorPayloadVersion = 44,
+ kErrorCodeUnsupportedMinorPayloadVersion = 45,
- // Note: When adding new error codes, please remember to add the
- // error into one of the buckets in PayloadState::UpdateFailed method so
- // that the retries across URLs and the payload backoff mechanism work
- // correctly for those new error codes.
+ // VERY IMPORTANT! When adding new error codes:
+ //
+ // 1) Update tools/metrics/histograms/histograms.xml in Chrome.
+ //
+ // 2) Update the assorted switch statements in update_engine which won't
+ // build until this case is added.
// Any code above this is sent to both Omaha and UMA as-is, except
// kErrorCodeOmahaErrorInHTTPResponse (see error code 2000 for more details).
diff --git a/payload_state.cc b/payload_state.cc
index 162848a..9839a2b 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -220,6 +220,8 @@
case kErrorCodeDownloadOperationHashMissingError:
case kErrorCodeDownloadMetadataSignatureMissingError:
case kErrorCodePayloadMismatchedType:
+ case kErrorCodeUnsupportedMajorPayloadVersion:
+ case kErrorCodeUnsupportedMinorPayloadVersion:
IncrementUrlIndex();
break;
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index ab59331..ec79950 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -90,7 +90,7 @@
class PayloadStateTest : public ::testing::Test { };
TEST(PayloadStateTest, DidYouAddANewErrorCode) {
- if (kErrorCodeUmaReportedMax != 44) {
+ if (kErrorCodeUmaReportedMax != 46) {
LOG(ERROR) << "The following failure is intentional. If you added a new "
<< "ErrorCode enum value, make sure to add it to the "
<< "PayloadState::UpdateFailed method and then update this test "
diff --git a/utils.cc b/utils.cc
index 5d32466..f23f31f 100644
--- a/utils.cc
+++ b/utils.cc
@@ -1000,6 +1000,10 @@
return "kErrorCodeSpecialFlags";
case kErrorCodePostinstallFirmwareRONotUpdatable:
return "kErrorCodePostinstallFirmwareRONotUpdatable";
+ case kErrorCodeUnsupportedMajorPayloadVersion:
+ return "kErrorCodeUnsupportedMajorPayloadVersion";
+ case kErrorCodeUnsupportedMinorPayloadVersion:
+ return "kErrorCodeUnsupportedMinorPayloadVersion";
// Don't add a default case to let the compiler warn about newly added
// error codes which should be added here.
}