HardwareInterface::IsPartitionUpdateValid: fine grained error
Let the function emit an error code instead of a boolean to indicate
details of the error that is encountered.
For every partition, if downgrade is detected, emit
kPayloadTimestampError. In this case, still check other partitions for
more severe errors before returning this error.
In some cases, e.g. DeltaArchiveManifest carries a version field that is
not a recognized format, or timestamp sysprops in Android is not an
integer, report a more severe error.
If only downgrade errors are encountered, AllowDowngrade() can still
override the result, and proceed with the update; but, AllowDowngrade
cannot override those severe errors.
Test: update_engine_unittest
Bug: 162623577
Bug: 162553432
Change-Id: Ifc2a6fcd66239c755fb4f6528c3d8c6848afcb27
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