HardwareInterface::IsPartitionUpdateValid: fine grained error am: 87029337e3
Original change: https://android-review.googlesource.com/c/platform/system/update_engine/+/1417653
Change-Id: I193c12083d3f8b7d785de207b6919417a62743af
diff --git a/common/fake_hardware.h b/common/fake_hardware.h
index 30c0897..30b5718 100644
--- a/common/fake_hardware.h
+++ b/common/fake_hardware.h
@@ -23,6 +23,7 @@
#include <base/time/time.h>
+#include "update_engine/common/error_code.h"
#include "update_engine/common/hardware_interface.h"
#include "update_engine/common/utils.h"
@@ -216,8 +217,9 @@
void SetVersion(const std::string& partition_name, std::string timestamp) {
partition_timestamps_[partition_name] = std::move(timestamp);
}
- bool IsPartitionUpdateValid(const std::string& partition_name,
- const std::string& new_version) const override {
+ ErrorCode IsPartitionUpdateValid(
+ const std::string& partition_name,
+ const std::string& new_version) const override {
const auto old_version = GetVersionForLogging(partition_name);
return utils::IsTimestampNewer(old_version, new_version);
}
diff --git a/common/hardware_interface.h b/common/hardware_interface.h
index 0fffbfb..b37b007 100644
--- a/common/hardware_interface.h
+++ b/common/hardware_interface.h
@@ -25,6 +25,8 @@
#include <base/files/file_path.h>
#include <base/time/time.h>
+#include "update_engine/common/error_code.h"
+
namespace chromeos_update_engine {
// The hardware interface allows access to the crossystem exposed properties,
@@ -153,8 +155,15 @@
// version number of partition `partition_name`. The notion of
// "newer" is defined by this function. Caller should not make
// any assumption about the underlying logic.
- virtual bool IsPartitionUpdateValid(const std::string& partition_name,
- const std::string& new_version) const = 0;
+ // Return:
+ // - kSuccess if update is valid.
+ // - kPayloadTimestampError if downgrade is detected
+ // - kDownloadManifestParseError if |new_version| has an incorrect format
+ // - Other error values if the source of error is known, or kError for
+ // a generic error on the device.
+ virtual ErrorCode IsPartitionUpdateValid(
+ const std::string& partition_name,
+ const std::string& new_version) const = 0;
};
} // namespace chromeos_update_engine
diff --git a/common/utils.cc b/common/utils.cc
index bbb155f..5d76f3f 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -990,19 +990,26 @@
return true;
}
-bool IsTimestampNewer(const std::string& old_version,
- const std::string& new_version) {
+ErrorCode IsTimestampNewer(const std::string& old_version,
+ const std::string& new_version) {
if (old_version.empty() || new_version.empty()) {
LOG(WARNING)
<< "One of old/new timestamp is empty, permit update anyway. Old: "
<< old_version << " New: " << new_version;
- return true;
+ return ErrorCode::kSuccess;
}
int64_t old_ver = 0;
- TEST_AND_RETURN_FALSE(ParseTimestamp(old_version, &old_ver));
+ if (!ParseTimestamp(old_version, &old_ver)) {
+ return ErrorCode::kError;
+ }
int64_t new_ver = 0;
- TEST_AND_RETURN_FALSE(ParseTimestamp(new_version, &new_ver));
- return old_ver <= new_ver;
+ if (!ParseTimestamp(new_version, &new_ver)) {
+ return ErrorCode::kDownloadManifestParseError;
+ }
+ if (old_ver > new_ver) {
+ return ErrorCode::kPayloadTimestampError;
+ }
+ return ErrorCode::kSuccess;
}
} // namespace utils
diff --git a/common/utils.h b/common/utils.h
index 5dfee3b..0a1dc0c 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -324,10 +324,15 @@
std::string GetExclusionName(const std::string& str_to_convert);
// Parse `old_version` and `new_version` as integer timestamps and
-// return true if `new_version` is larger/newer.
-// Returns true if either one is empty. Return false if
-bool IsTimestampNewer(const std::string& old_version,
- const std::string& new_version);
+// Return kSuccess if `new_version` is larger/newer.
+// Return kSuccess if either one is empty.
+// Return kError if |old_version| is not empty and not an integer.
+// Return kDownloadManifestParseError if |new_version| is not empty and not an
+// integer.
+// Return kPayloadTimestampError if both are integers but |new_version| <
+// |old_version|.
+ErrorCode IsTimestampNewer(const std::string& old_version,
+ const std::string& new_version);
} // namespace utils
diff --git a/common/utils_unittest.cc b/common/utils_unittest.cc
index 37871d2..d73b3da 100644
--- a/common/utils_unittest.cc
+++ b/common/utils_unittest.cc
@@ -482,11 +482,13 @@
}
TEST(UtilsTest, ValidatePerPartitionTimestamp) {
- ASSERT_FALSE(utils::IsTimestampNewer("10", "5"));
- ASSERT_TRUE(utils::IsTimestampNewer("10", "11"));
- ASSERT_FALSE(utils::IsTimestampNewer("10", "lol"));
- ASSERT_FALSE(utils::IsTimestampNewer("lol", "ZZZ"));
- ASSERT_TRUE(utils::IsTimestampNewer("10", ""));
+ ASSERT_EQ(ErrorCode::kPayloadTimestampError,
+ utils::IsTimestampNewer("10", "5"));
+ ASSERT_EQ(ErrorCode::kSuccess, utils::IsTimestampNewer("10", "11"));
+ ASSERT_EQ(ErrorCode::kDownloadManifestParseError,
+ utils::IsTimestampNewer("10", "lol"));
+ ASSERT_EQ(ErrorCode::kError, utils::IsTimestampNewer("lol", "ZZZ"));
+ ASSERT_EQ(ErrorCode::kSuccess, utils::IsTimestampNewer("10", ""));
}
} // namespace chromeos_update_engine
diff --git a/hardware_android.cc b/hardware_android.cc
index 659e67e..361b9f1 100644
--- a/hardware_android.cc
+++ b/hardware_android.cc
@@ -27,6 +27,7 @@
#include <base/files/file_util.h>
#include <bootloader_message/bootloader_message.h>
+#include "update_engine/common/error_code_utils.h"
#include "update_engine/common/hardware.h"
#include "update_engine/common/platform_constants.h"
#include "update_engine/common/utils.h"
@@ -233,18 +234,19 @@
"");
}
-bool HardwareAndroid::IsPartitionUpdateValid(
+ErrorCode HardwareAndroid::IsPartitionUpdateValid(
const std::string& partition_name, const std::string& new_version) const {
const auto old_version = GetVersionForLogging(partition_name);
// TODO(zhangkelvin) for some partitions, missing a current timestamp should
// be an error, e.g. system, vendor, product etc.
- auto applicable = utils::IsTimestampNewer(old_version, new_version);
- if (!applicable) {
- LOG(ERROR) << "Timestamp on partition " << partition_name
- << " is newer than update. Partition timestamp: " << old_version
+ auto error_code = utils::IsTimestampNewer(old_version, new_version);
+ if (error_code != ErrorCode::kSuccess) {
+ LOG(ERROR) << "Timestamp check failed with "
+ << utils::ErrorCodeToString(error_code)
+ << " Partition timestamp: " << old_version
<< " Update timestamp: " << new_version;
}
- return applicable;
+ return error_code;
}
} // namespace chromeos_update_engine
diff --git a/hardware_android.h b/hardware_android.h
index 2e55f97..d8fbbbe 100644
--- a/hardware_android.h
+++ b/hardware_android.h
@@ -23,6 +23,7 @@
#include <base/macros.h>
#include <base/time/time.h>
+#include "update_engine/common/error_code.h"
#include "update_engine/common/hardware.h"
#include "update_engine/common/hardware_interface.h"
@@ -61,7 +62,7 @@
void SetWarmReset(bool warm_reset) override;
[[nodiscard]] std::string GetVersionForLogging(
const std::string& partition_name) const override;
- [[nodiscard]] bool IsPartitionUpdateValid(
+ [[nodiscard]] ErrorCode IsPartitionUpdateValid(
const std::string& partition_name,
const std::string& new_version) const override;
diff --git a/hardware_chromeos.cc b/hardware_chromeos.cc
index 58f30db..807e086 100644
--- a/hardware_chromeos.cc
+++ b/hardware_chromeos.cc
@@ -389,10 +389,11 @@
// TODO(zhangkelvin) Implement per-partition timestamp for Chrome OS.
return "";
}
-bool HardwareChromeOS::IsPartitionUpdateValid(
+
+ErrorCode HardwareChromeOS::IsPartitionUpdateValid(
const std::string& partition_name, const std::string& new_version) const {
// TODO(zhangkelvin) Implement per-partition timestamp for Chrome OS.
- return true;
+ return ErrorCode::kSuccess;
}
} // namespace chromeos_update_engine
diff --git a/hardware_chromeos.h b/hardware_chromeos.h
index 49fed88..bbfe273 100644
--- a/hardware_chromeos.h
+++ b/hardware_chromeos.h
@@ -25,6 +25,7 @@
#include <base/time/time.h>
#include <debugd/dbus-proxies.h>
+#include "update_engine/common/error_code.h"
#include "update_engine/common/hardware_interface.h"
namespace chromeos_update_engine {
@@ -65,8 +66,9 @@
void SetWarmReset(bool warm_reset) override;
std::string GetVersionForLogging(
const std::string& partition_name) const override;
- bool IsPartitionUpdateValid(const std::string& partition_name,
- const std::string& new_version) const override;
+ ErrorCode IsPartitionUpdateValid(
+ const std::string& partition_name,
+ const std::string& new_version) const override;
private:
friend class HardwareChromeOSTest;
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index aa0b4f5..ba96047 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -41,6 +41,8 @@
#include <puffin/puffpatch.h>
#include "update_engine/common/constants.h"
+#include "update_engine/common/error_code.h"
+#include "update_engine/common/error_code_utils.h"
#include "update_engine/common/hardware_interface.h"
#include "update_engine/common/prefs_interface.h"
#include "update_engine/common/subprocess.h"
@@ -1628,15 +1630,19 @@
LOG(ERROR) << "Manifest contains deprecated fields.";
return ErrorCode::kPayloadMismatchedType;
}
- TimestampCheckResult result = CheckTimestampError();
- if (result == TimestampCheckResult::DOWNGRADE) {
- if (!hardware_->AllowDowngrade()) {
- return ErrorCode::kPayloadTimestampError;
+ ErrorCode error_code = CheckTimestampError();
+ if (error_code != ErrorCode::kSuccess) {
+ if (error_code == ErrorCode::kPayloadTimestampError) {
+ if (!hardware_->AllowDowngrade()) {
+ return ErrorCode::kPayloadTimestampError;
+ }
+ LOG(INFO) << "The current OS build allows downgrade, continuing to apply"
+ " the payload with an older timestamp.";
+ } else {
+ LOG(ERROR) << "Timestamp check returned "
+ << utils::ErrorCodeToString(error_code);
+ return error_code;
}
- LOG(INFO) << "The current OS build allows downgrade, continuing to apply"
- " the payload with an older timestamp.";
- } else if (result == TimestampCheckResult::FAILURE) {
- return ErrorCode::kPayloadTimestampError;
}
// TODO(crbug.com/37661) we should be adding more and more manifest checks,
@@ -1645,51 +1651,87 @@
return ErrorCode::kSuccess;
}
-TimestampCheckResult DeltaPerformer::CheckTimestampError() const {
+ErrorCode DeltaPerformer::CheckTimestampError() const {
bool is_partial_update =
manifest_.has_partial_update() && manifest_.partial_update();
const auto& partitions = manifest_.partitions();
- auto&& timestamp_valid = [this](const PartitionUpdate& partition) {
- return hardware_->IsPartitionUpdateValid(partition.partition_name(),
- partition.version());
+
+ // Check version field for a given PartitionUpdate object. If an error
+ // is encountered, set |error_code| accordingly. If downgrade is detected,
+ // |downgrade_detected| is set. Return true if the program should continue to
+ // check the next partition or not, or false if it should exit early due to
+ // errors.
+ auto&& timestamp_valid = [this](const PartitionUpdate& partition,
+ bool allow_empty_version,
+ bool* downgrade_detected) -> ErrorCode {
+ if (!partition.has_version()) {
+ if (allow_empty_version) {
+ return ErrorCode::kSuccess;
+ }
+ LOG(ERROR)
+ << "PartitionUpdate " << partition.partition_name()
+ << " does ot have a version field. Not allowed in partial updates.";
+ return ErrorCode::kDownloadManifestParseError;
+ }
+
+ auto error_code = hardware_->IsPartitionUpdateValid(
+ partition.partition_name(), partition.version());
+ switch (error_code) {
+ case ErrorCode::kSuccess:
+ break;
+ case ErrorCode::kPayloadTimestampError:
+ *downgrade_detected = true;
+ LOG(WARNING) << "PartitionUpdate " << partition.partition_name()
+ << " has an older version than partition on device.";
+ break;
+ default:
+ LOG(ERROR) << "IsPartitionUpdateValid(" << partition.partition_name()
+ << ") returned" << utils::ErrorCodeToString(error_code);
+ break;
+ }
+ return error_code;
};
+
+ bool downgrade_detected = false;
+
if (is_partial_update) {
// for partial updates, all partition MUST have valid timestamps
// But max_timestamp can be empty
for (const auto& partition : partitions) {
- if (!partition.has_version()) {
- LOG(ERROR)
- << "PartitionUpdate " << partition.partition_name()
- << " does ot have a version field. Not allowed in partial updates.";
- return TimestampCheckResult::FAILURE;
- }
- if (!timestamp_valid(partition)) {
- // Warning because the system might allow downgrade.
- LOG(WARNING) << "PartitionUpdate " << partition.partition_name()
- << " has an older version than partition on device.";
- return TimestampCheckResult::DOWNGRADE;
+ auto error_code = timestamp_valid(
+ partition, false /* allow_empty_version */, &downgrade_detected);
+ if (error_code != ErrorCode::kSuccess &&
+ error_code != ErrorCode::kPayloadTimestampError) {
+ return error_code;
}
}
-
- return TimestampCheckResult::SUCCESS;
+ if (downgrade_detected) {
+ return ErrorCode::kPayloadTimestampError;
+ }
+ return ErrorCode::kSuccess;
}
+
+ // For non-partial updates, check max_timestamp first.
if (manifest_.max_timestamp() < hardware_->GetBuildTimestamp()) {
LOG(ERROR) << "The current OS build timestamp ("
<< hardware_->GetBuildTimestamp()
<< ") is newer than the maximum timestamp in the manifest ("
<< manifest_.max_timestamp() << ")";
- return TimestampCheckResult::DOWNGRADE;
+ return ErrorCode::kPayloadTimestampError;
}
// Otherwise... partitions can have empty timestamps.
for (const auto& partition : partitions) {
- if (partition.has_version() && !timestamp_valid(partition)) {
- // Warning because the system might allow downgrade.
- LOG(WARNING) << "PartitionUpdate " << partition.partition_name()
- << " has an older version than partition on device.";
- return TimestampCheckResult::DOWNGRADE;
+ auto error_code = timestamp_valid(
+ partition, true /* allow_empty_version */, &downgrade_detected);
+ if (error_code != ErrorCode::kSuccess &&
+ error_code != ErrorCode::kPayloadTimestampError) {
+ return error_code;
}
}
- return TimestampCheckResult::SUCCESS;
+ if (downgrade_detected) {
+ return ErrorCode::kPayloadTimestampError;
+ }
+ return ErrorCode::kSuccess;
}
ErrorCode DeltaPerformer::ValidateOperationHash(
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index 0718ef6..88076af 100644
--- a/payload_consumer/delta_performer.h
+++ b/payload_consumer/delta_performer.h
@@ -48,13 +48,6 @@
// This class performs the actions in a delta update synchronously. The delta
// update itself should be passed in in chunks as it is received.
-
-enum class TimestampCheckResult {
- SUCCESS,
- FAILURE,
- DOWNGRADE,
-};
-
class DeltaPerformer : public FileWriter {
public:
// Defines the granularity of progress logging in terms of how many "completed
@@ -316,9 +309,14 @@
// Also see comment for the static PreparePartitionsForUpdate().
bool PreparePartitionsForUpdate(uint64_t* required_size);
- // Check if current manifest contains timestamp errors. (ill-formed or
- // downgrade)
- TimestampCheckResult CheckTimestampError() const;
+ // Check if current manifest contains timestamp errors.
+ // Return:
+ // - kSuccess if update is valid.
+ // - kPayloadTimestampError if downgrade is detected
+ // - kDownloadManifestParseError if |new_version| has an incorrect format
+ // - Other error values if the source of error is known, or kError for
+ // a generic error on the device.
+ ErrorCode CheckTimestampError() const;
// Update Engine preference store.
PrefsInterface* prefs_;
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index c257b28..d2e0f6c 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -1206,28 +1206,68 @@
}
TEST_F(DeltaPerformerIntegrationTest,
- ValidatePerPartitionTimestampPartialUpdate) {
- // The Manifest we are validating.
- DeltaArchiveManifest manifest;
-
+ ValidatePerPartitionTimestampPartialUpdatePass) {
fake_hardware_.SetVersion("system", "5");
fake_hardware_.SetVersion("product", "99");
- fake_hardware_.SetBuildTimestamp(1);
+ DeltaArchiveManifest manifest;
manifest.set_minor_version(kPartialUpdateMinorPayloadVersion);
- manifest.set_max_timestamp(2);
manifest.set_partial_update(true);
- AddPartition(&manifest, "system", 10);
- {
- auto& partition = *manifest.add_partitions();
- // For partial updates, missing timestamp should
- // trigger an error
- partition.set_partition_name("product");
- }
+ AddPartition(&manifest, "product", 100);
+ RunManifestValidation(
+ manifest, kMaxSupportedMajorPayloadVersion, ErrorCode::kSuccess);
+}
+TEST_F(DeltaPerformerIntegrationTest,
+ ValidatePerPartitionTimestampPartialUpdateDowngrade) {
+ fake_hardware_.SetVersion("system", "5");
+ fake_hardware_.SetVersion("product", "99");
+
+ DeltaArchiveManifest manifest;
+ manifest.set_minor_version(kPartialUpdateMinorPayloadVersion);
+ manifest.set_partial_update(true);
+ AddPartition(&manifest, "product", 98);
RunManifestValidation(manifest,
kMaxSupportedMajorPayloadVersion,
ErrorCode::kPayloadTimestampError);
}
+TEST_F(DeltaPerformerIntegrationTest,
+ ValidatePerPartitionTimestampPartialUpdateMissingVersion) {
+ fake_hardware_.SetVersion("system", "5");
+ fake_hardware_.SetVersion("product", "99");
+
+ DeltaArchiveManifest manifest;
+ manifest.set_minor_version(kPartialUpdateMinorPayloadVersion);
+ manifest.set_partial_update(true);
+ {
+ auto& partition = *manifest.add_partitions();
+ // For partial updates, missing timestamp should trigger an error
+ partition.set_partition_name("product");
+ // has_version() == false.
+ }
+ RunManifestValidation(manifest,
+ kMaxSupportedMajorPayloadVersion,
+ ErrorCode::kDownloadManifestParseError);
+}
+
+TEST_F(DeltaPerformerIntegrationTest,
+ ValidatePerPartitionTimestampPartialUpdateEmptyVersion) {
+ fake_hardware_.SetVersion("system", "5");
+ fake_hardware_.SetVersion("product", "99");
+
+ DeltaArchiveManifest manifest;
+ manifest.set_minor_version(kPartialUpdateMinorPayloadVersion);
+ manifest.set_partial_update(true);
+ {
+ auto& partition = *manifest.add_partitions();
+ // For partial updates, missing timestamp should trigger an error
+ partition.set_partition_name("product");
+ partition.set_version("something");
+ }
+ RunManifestValidation(manifest,
+ kMaxSupportedMajorPayloadVersion,
+ ErrorCode::kDownloadManifestParseError);
+}
+
} // namespace chromeos_update_engine