update_engine: Don't do force updates after rollback.
If the device was rolled back via enterprise policy since the
last update check where policy was present, we shouldn't do a
forced update, because this could lead to an infinite
rollback-update loop.
This is achieved by adding a new pref 'rollback-happened' to
'powerwash_safe_prefs'. This creates a file at '/mnt/
stateful_partition/unencrypted/preserve/update_engine/prefs/
rollback-happened', which is not deleted during powerwash
(crrev.com/c/1005180). If this file exists after powerwash and
its content is 'true', it means the device was rolled back, and
forced updates must not be applied before the device is enrolled
and policy is available again.
When an update check happens and policy is available or device
becomes a consumer device, the file is deleted.
BUG=chromium:831266
TEST='cros_run_unit_tests --board=cyan --packages update_engine'
Change-Id: I0a06696701530c93d4ed388c42e43b65fe78f777
Reviewed-on: https://chromium-review.googlesource.com/1005181
Commit-Ready: Marton Hunyady <hunyadym@chromium.org>
Tested-by: Marton Hunyady <hunyadym@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/common/constants.cc b/common/constants.cc
index d3d851a..215ca31 100644
--- a/common/constants.cc
+++ b/common/constants.cc
@@ -61,6 +61,7 @@
const char kPrefsPostInstallSucceeded[] = "post-install-succeeded";
const char kPrefsPreviousVersion[] = "previous-version";
const char kPrefsResumedUpdateFailures[] = "resumed-update-failures";
+const char kPrefsRollbackHappened[] = "rollback-happened";
const char kPrefsRollbackVersion[] = "rollback-version";
const char kPrefsChannelOnSlotPrefix[] = "channel-on-slot-";
const char kPrefsSystemUpdatedMarker[] = "system-updated-marker";
diff --git a/common/constants.h b/common/constants.h
index ddf7df5..a3d01c9 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -63,6 +63,7 @@
extern const char kPrefsPostInstallSucceeded[];
extern const char kPrefsPreviousVersion[];
extern const char kPrefsResumedUpdateFailures[];
+extern const char kPrefsRollbackHappened[];
extern const char kPrefsRollbackVersion[];
extern const char kPrefsChannelOnSlotPrefix[];
extern const char kPrefsSystemUpdatedMarker[];
diff --git a/mock_payload_state.h b/mock_payload_state.h
index 6dccc64..4cfcac0 100644
--- a/mock_payload_state.h
+++ b/mock_payload_state.h
@@ -68,6 +68,8 @@
MOCK_METHOD1(GetCurrentBytesDownloaded, uint64_t(DownloadSource source));
MOCK_METHOD1(GetTotalBytesDownloaded, uint64_t(DownloadSource source));
MOCK_METHOD0(GetNumReboots, uint32_t());
+ MOCK_METHOD0(GetRollbackHappened, bool());
+ MOCK_METHOD1(SetRollbackHappened, void(bool));
MOCK_METHOD0(GetRollbackVersion, std::string());
MOCK_METHOD0(GetP2PNumAttempts, int());
MOCK_METHOD0(GetP2PFirstAttemptTimestamp, base::Time());
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 2d6105a..744a5c8 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -156,9 +156,16 @@
// method and UpdateStatus signal. A potential issue is that update_engine may
// be unresponsive during an update download.
if (!deadline_file_.empty()) {
- utils::WriteFile(deadline_file_.c_str(),
- response.deadline.data(),
- response.deadline.size());
+ if (payload_state->GetRollbackHappened()) {
+ // Don't do forced update if rollback has happened since the last update
+ // check where policy was present.
+ LOG(INFO) << "Not forcing update because a rollback happened.";
+ utils::WriteFile(deadline_file_.c_str(), nullptr, 0);
+ } else {
+ utils::WriteFile(deadline_file_.c_str(),
+ response.deadline.data(),
+ response.deadline.size());
+ }
chmod(deadline_file_.c_str(), S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
}
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 9e2cdd1..aba71a2 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -223,6 +223,34 @@
in.deadline = "some-deadline";
InstallPlan install_plan;
fake_system_state_.fake_boot_control()->SetCurrentSlot(0);
+ // Because rollback happened, the deadline shouldn't be written into the
+ // file.
+ EXPECT_CALL(*(fake_system_state_.mock_payload_state()),
+ GetRollbackHappened())
+ .WillOnce(Return(true));
+ EXPECT_TRUE(DoTest(in, test_deadline_file, &install_plan));
+ EXPECT_EQ(in.packages[0].payload_urls[0], install_plan.download_url);
+ EXPECT_EQ(expected_hash_, install_plan.payloads[0].hash);
+ EXPECT_EQ(1U, install_plan.target_slot);
+ string deadline;
+ EXPECT_TRUE(utils::ReadFile(test_deadline_file, &deadline));
+ EXPECT_TRUE(deadline.empty());
+ EXPECT_EQ(in.version, install_plan.version);
+ }
+ {
+ OmahaResponse in;
+ in.update_exists = true;
+ in.version = "a.b.c.d";
+ in.packages.push_back(
+ {.payload_urls = {kLongName}, .size = 12, .hash = kPayloadHashHex});
+ in.more_info_url = "http://more/info";
+ in.prompt = true;
+ in.deadline = "some-deadline";
+ InstallPlan install_plan;
+ fake_system_state_.fake_boot_control()->SetCurrentSlot(0);
+ EXPECT_CALL(*(fake_system_state_.mock_payload_state()),
+ GetRollbackHappened())
+ .WillOnce(Return(false));
EXPECT_TRUE(DoTest(in, test_deadline_file, &install_plan));
EXPECT_EQ(in.packages[0].payload_urls[0], install_plan.download_url);
EXPECT_EQ(expected_hash_, install_plan.payloads[0].hash);
diff --git a/payload_state.cc b/payload_state.cc
index a393b91..745ae2e 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -64,6 +64,7 @@
url_index_(0),
url_failure_count_(0),
url_switch_count_(0),
+ rollback_happened_(false),
attempt_num_bytes_downloaded_(0),
attempt_connection_type_(metrics::ConnectionType::kUnknown),
attempt_error_code_(ErrorCode::kSuccess),
@@ -94,6 +95,7 @@
}
LoadNumReboots();
LoadNumResponsesSeen();
+ LoadRollbackHappened();
LoadRollbackVersion();
LoadP2PFirstAttemptTimestamp();
LoadP2PNumAttempts();
@@ -1072,6 +1074,25 @@
SetNumReboots(GetPersistedValue(kPrefsNumReboots, prefs_));
}
+void PayloadState::LoadRollbackHappened() {
+ CHECK(powerwash_safe_prefs_);
+ bool rollback_happened = false;
+ powerwash_safe_prefs_->GetBoolean(kPrefsRollbackHappened, &rollback_happened);
+ SetRollbackHappened(rollback_happened);
+}
+
+void PayloadState::SetRollbackHappened(bool rollback_happened) {
+ CHECK(powerwash_safe_prefs_);
+ LOG(INFO) << "Setting rollback-happened to " << rollback_happened << ".";
+ rollback_happened_ = rollback_happened;
+ if (rollback_happened) {
+ powerwash_safe_prefs_->SetBoolean(kPrefsRollbackHappened,
+ rollback_happened);
+ } else {
+ powerwash_safe_prefs_->Delete(kPrefsRollbackHappened);
+ }
+}
+
void PayloadState::LoadRollbackVersion() {
CHECK(powerwash_safe_prefs_);
string rollback_version;
diff --git a/payload_state.h b/payload_state.h
index 4ec5bf9..0868a10 100644
--- a/payload_state.h
+++ b/payload_state.h
@@ -120,6 +120,10 @@
void UpdateEngineStarted() override;
+ inline bool GetRollbackHappened() override { return rollback_happened_; }
+
+ void SetRollbackHappened(bool rollback_happened) override;
+
inline std::string GetRollbackVersion() override {
return rollback_version_;
}
@@ -167,6 +171,7 @@
FRIEND_TEST(PayloadStateTest, RebootAfterUpdateFailedMetric);
FRIEND_TEST(PayloadStateTest, RebootAfterUpdateSucceed);
FRIEND_TEST(PayloadStateTest, RebootAfterCanceledUpdate);
+ FRIEND_TEST(PayloadStateTest, RollbackHappened);
FRIEND_TEST(PayloadStateTest, RollbackVersion);
FRIEND_TEST(PayloadStateTest, UpdateSuccessWithWipedPrefs);
@@ -364,6 +369,10 @@
uint64_t total_bytes_downloaded,
bool log);
+ // Loads whether rollback has happened on this device since the last update
+ // check where policy was available. This info is preserved over powerwash.
+ void LoadRollbackHappened();
+
// Loads the blacklisted version from our prefs file.
void LoadRollbackVersion();
@@ -552,6 +561,11 @@
// allowed as per device policy.
std::vector<std::vector<std::string>> candidate_urls_;
+ // This stores whether rollback has happened since the last time device policy
+ // was available during update check. When this is set, we're preventing
+ // forced updates to avoid update-rollback loops.
+ bool rollback_happened_;
+
// This stores a blacklisted version set as part of rollback. When we rollback
// we store the version of the os from which we are rolling back from in order
// to guarantee that we do not re-update to it on the next au attempt after
diff --git a/payload_state_interface.h b/payload_state_interface.h
index 4aa25e3..2460d24 100644
--- a/payload_state_interface.h
+++ b/payload_state_interface.h
@@ -155,6 +155,16 @@
// Called at update_engine startup to do various house-keeping.
virtual void UpdateEngineStarted() = 0;
+ // Returns whether a rollback happened since the last update check with policy
+ // present.
+ virtual bool GetRollbackHappened() = 0;
+
+ // Sets whether rollback has happened on this device since the last update
+ // check where policy was available. This info is preserved over powerwash.
+ // This prevents forced updates happening on a rolled back device before
+ // device policy is available.
+ virtual void SetRollbackHappened(bool rollback_happened) = 0;
+
// Returns the version from before a rollback if our last update was a
// rollback.
virtual std::string GetRollbackVersion() = 0;
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index f1c3835..bcb196a 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -984,6 +984,37 @@
EXPECT_EQ(0U, payload_state.GetNumReboots());
}
+TEST(PayloadStateTest, RollbackHappened) {
+ FakeSystemState fake_system_state;
+ PayloadState payload_state;
+
+ NiceMock<MockPrefs>* mock_powerwash_safe_prefs =
+ fake_system_state.mock_powerwash_safe_prefs();
+ EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+ // Verify pre-conditions are good.
+ EXPECT_FALSE(payload_state.GetRollbackHappened());
+
+ // Set to true.
+ EXPECT_CALL(*mock_powerwash_safe_prefs,
+ SetBoolean(kPrefsRollbackHappened, true));
+ payload_state.SetRollbackHappened(true);
+ EXPECT_TRUE(payload_state.GetRollbackHappened());
+
+ // Set to false.
+ EXPECT_CALL(*mock_powerwash_safe_prefs, Delete(kPrefsRollbackHappened));
+ payload_state.SetRollbackHappened(false);
+ EXPECT_FALSE(payload_state.GetRollbackHappened());
+
+ // Let's verify we can reload it correctly.
+ EXPECT_CALL(*mock_powerwash_safe_prefs, GetBoolean(kPrefsRollbackHappened, _))
+ .WillOnce(DoAll(SetArgPointee<1>(true), Return(true)));
+ EXPECT_CALL(*mock_powerwash_safe_prefs,
+ SetBoolean(kPrefsRollbackHappened, true));
+ payload_state.LoadRollbackHappened();
+ EXPECT_TRUE(payload_state.GetRollbackHappened());
+}
+
TEST(PayloadStateTest, RollbackVersion) {
FakeSystemState fake_system_state;
PayloadState payload_state;
diff --git a/update_attempter.cc b/update_attempter.cc
index ebf7fb0..10118ac 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -62,6 +62,7 @@
#include "update_engine/power_manager_interface.h"
#include "update_engine/system_state.h"
#include "update_engine/update_manager/policy.h"
+#include "update_engine/update_manager/policy_utils.h"
#include "update_engine/update_manager/update_manager.h"
#include "update_engine/update_status_utils.h"
@@ -361,6 +362,10 @@
// Refresh the policy before computing all the update parameters.
RefreshDevicePolicy();
+ // Check whether we need to clear the rollback-happened preference after
+ // policy is available again.
+ UpdateRollbackHappened();
+
// Update the target version prefix.
omaha_request_params_->set_target_version_prefix(target_version_prefix);
@@ -904,6 +909,19 @@
last_checked_time_ = system_state_->clock()->GetWallclockTime().ToTimeT();
}
+void UpdateAttempter::UpdateRollbackHappened() {
+ DCHECK(system_state_);
+ DCHECK(system_state_->payload_state());
+ DCHECK(policy_provider_);
+ if (system_state_->payload_state()->GetRollbackHappened() &&
+ (policy_provider_->device_policy_is_loaded() ||
+ policy_provider_->IsConsumerDevice())) {
+ // Rollback happened, but we already went through OOBE and policy is
+ // present or it's a consumer device.
+ system_state_->payload_state()->SetRollbackHappened(false);
+ }
+}
+
// Delegate methods:
void UpdateAttempter::ProcessingDone(const ActionProcessor* processor,
ErrorCode code) {
diff --git a/update_attempter.h b/update_attempter.h
index 76e93a2..24376e9 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -396,6 +396,10 @@
// Updates the time an update was last attempted to the current time.
void UpdateLastCheckedTime();
+ // Checks whether we need to clear the rollback-happened preference after
+ // policy is available again.
+ void UpdateRollbackHappened();
+
// Returns whether an update is currently running or scheduled.
bool IsUpdateRunningOrScheduled();
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index dd61fa5..9dfc008 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -29,6 +29,7 @@
#include <gtest/gtest.h>
#include <policy/libpolicy.h>
#include <policy/mock_device_policy.h>
+#include <policy/mock_libpolicy.h>
#include "update_engine/common/fake_clock.h"
#include "update_engine/common/fake_prefs.h"
@@ -64,6 +65,7 @@
using testing::Property;
using testing::Return;
using testing::ReturnPointee;
+using testing::ReturnRef;
using testing::SaveArg;
using testing::SetArgPointee;
using update_engine::UpdateAttemptFlags;
@@ -166,6 +168,9 @@
void P2PEnabledInteractiveStart();
void P2PEnabledStartingFailsStart();
void P2PEnabledHousekeepingFailsStart();
+ void ResetRollbackHappenedStart(bool is_consumer,
+ bool is_policy_available,
+ bool expected_reset);
bool actual_using_p2p_for_downloading() {
return actual_using_p2p_for_downloading_;
@@ -1140,4 +1145,56 @@
attempter_.GetCurrentUpdateAttemptFlags());
}
+void UpdateAttempterTest::ResetRollbackHappenedStart(bool is_consumer,
+ bool is_policy_loaded,
+ bool expected_reset) {
+ EXPECT_CALL(*fake_system_state_.mock_payload_state(), GetRollbackHappened())
+ .WillRepeatedly(Return(true));
+ auto mock_policy_provider =
+ std::make_unique<NiceMock<policy::MockPolicyProvider>>();
+ EXPECT_CALL(*mock_policy_provider, IsConsumerDevice())
+ .WillRepeatedly(Return(is_consumer));
+ EXPECT_CALL(*mock_policy_provider, device_policy_is_loaded())
+ .WillRepeatedly(Return(is_policy_loaded));
+ const policy::MockDevicePolicy device_policy;
+ EXPECT_CALL(*mock_policy_provider, GetDevicePolicy())
+ .WillRepeatedly(ReturnRef(device_policy));
+ EXPECT_CALL(*fake_system_state_.mock_payload_state(),
+ SetRollbackHappened(false))
+ .Times(expected_reset ? 1 : 0);
+ attempter_.policy_provider_ = std::move(mock_policy_provider);
+ attempter_.Update("", "", "", "", false, /*interactive=*/true);
+ ScheduleQuitMainLoop();
+}
+
+TEST_F(UpdateAttempterTest, ResetRollbackHappenedOobe) {
+ loop_.PostTask(FROM_HERE,
+ base::Bind(&UpdateAttempterTest::ResetRollbackHappenedStart,
+ base::Unretained(this),
+ /*is_consumer=*/false,
+ /*is_policy_loaded=*/false,
+ /*expected_reset=*/false));
+ loop_.Run();
+}
+
+TEST_F(UpdateAttempterTest, ResetRollbackHappenedConsumer) {
+ loop_.PostTask(FROM_HERE,
+ base::Bind(&UpdateAttempterTest::ResetRollbackHappenedStart,
+ base::Unretained(this),
+ /*is_consumer=*/true,
+ /*is_policy_loaded=*/false,
+ /*expected_reset=*/true));
+ loop_.Run();
+}
+
+TEST_F(UpdateAttempterTest, ResetRollbackHappenedEnterprise) {
+ loop_.PostTask(FROM_HERE,
+ base::Bind(&UpdateAttempterTest::ResetRollbackHappenedStart,
+ base::Unretained(this),
+ /*is_consumer=*/false,
+ /*is_policy_loaded=*/true,
+ /*expected_reset=*/true));
+ loop_.Run();
+}
+
} // namespace chromeos_update_engine